From 6de94cb3d1c079cb19a96b0ec7d79cd73eb8947f Mon Sep 17 00:00:00 2001 From: Martin Sirringhaus Date: Wed, 4 Mar 2026 10:44:08 +0100 Subject: [PATCH 1/2] Make PinNotSet errors an interactive flow --- .../examples/authenticator_config_hid.rs | 2 + libwebauthn/examples/bio_enrollment_hid.rs | 2 + libwebauthn/examples/change_pin_hid.rs | 2 + libwebauthn/examples/cred_management.rs | 2 + libwebauthn/examples/prf_test.rs | 2 + libwebauthn/examples/webauthn_cable.rs | 2 + .../examples/webauthn_extensions_hid.rs | 2 + libwebauthn/examples/webauthn_hid.rs | 35 ++- libwebauthn/examples/webauthn_json_hid.rs | 2 + libwebauthn/examples/webauthn_nfc.rs | 35 ++- .../examples/webauthn_preflight_hid.rs | 2 + libwebauthn/examples/webauthn_prf_hid.rs | 2 + libwebauthn/src/lib.rs | 38 ++- libwebauthn/src/pin.rs | 46 +++- libwebauthn/src/webauthn/pin_uv_auth_token.rs | 218 ++++++++++++++---- 15 files changed, 344 insertions(+), 48 deletions(-) diff --git a/libwebauthn/examples/authenticator_config_hid.rs b/libwebauthn/examples/authenticator_config_hid.rs index d18df4be..d25c5f77 100644 --- a/libwebauthn/examples/authenticator_config_hid.rs +++ b/libwebauthn/examples/authenticator_config_hid.rs @@ -27,6 +27,8 @@ async fn handle_updates(mut state_recv: Receiver) { while let Ok(update) = state_recv.recv().await { match update { UvUpdate::PresenceRequired => println!("Please touch your device!"), + // Shouldn't really happen, as we don't use UV required + UvUpdate::PinNotSet(_) => println!("Pin not set for your device!"), UvUpdate::UvRetry { attempts_left } => { print!("UV failed."); if let Some(attempts_left) = attempts_left { diff --git a/libwebauthn/examples/bio_enrollment_hid.rs b/libwebauthn/examples/bio_enrollment_hid.rs index 62f4443d..2bd2a5e3 100644 --- a/libwebauthn/examples/bio_enrollment_hid.rs +++ b/libwebauthn/examples/bio_enrollment_hid.rs @@ -27,6 +27,8 @@ async fn handle_updates(mut state_recv: Receiver) { while let Ok(update) = state_recv.recv().await { match update { UvUpdate::PresenceRequired => println!("Please touch your device!"), + // Shouldn't really happen, as we don't use UV required + UvUpdate::PinNotSet(_) => println!("Pin not set for your device!"), UvUpdate::UvRetry { attempts_left } => { print!("UV failed."); if let Some(attempts_left) = attempts_left { diff --git a/libwebauthn/examples/change_pin_hid.rs b/libwebauthn/examples/change_pin_hid.rs index 4b38a13b..459bdfa1 100644 --- a/libwebauthn/examples/change_pin_hid.rs +++ b/libwebauthn/examples/change_pin_hid.rs @@ -28,6 +28,8 @@ async fn handle_updates(mut state_recv: Receiver) { while let Ok(update) = state_recv.recv().await { match update { UvUpdate::PresenceRequired => println!("Please touch your device!"), + // Can't happen, as we are right now trying to set the PIN + UvUpdate::PinNotSet(_) => panic!("Pin not set for your device!"), UvUpdate::UvRetry { attempts_left } => { print!("UV failed."); if let Some(attempts_left) = attempts_left { diff --git a/libwebauthn/examples/cred_management.rs b/libwebauthn/examples/cred_management.rs index f10dab5d..d6bbb42f 100644 --- a/libwebauthn/examples/cred_management.rs +++ b/libwebauthn/examples/cred_management.rs @@ -28,6 +28,8 @@ async fn handle_updates(mut state_recv: Receiver) { while let Ok(update) = state_recv.recv().await { match update { UvUpdate::PresenceRequired => println!("Please touch your device!"), + // Shouldn't really happen, as we don't use UV required + UvUpdate::PinNotSet(_) => println!("Pin not set for your device!"), UvUpdate::UvRetry { attempts_left } => { print!("UV failed."); if let Some(attempts_left) = attempts_left { diff --git a/libwebauthn/examples/prf_test.rs b/libwebauthn/examples/prf_test.rs index db5afbab..577938b7 100644 --- a/libwebauthn/examples/prf_test.rs +++ b/libwebauthn/examples/prf_test.rs @@ -35,6 +35,8 @@ async fn handle_updates(mut state_recv: Receiver) { while let Ok(update) = state_recv.recv().await { match update { UvUpdate::PresenceRequired => println!("Please touch your device!"), + // Shouldn't really happen, as we don't use UV required + UvUpdate::PinNotSet(_) => println!("Pin not set for your device!"), UvUpdate::UvRetry { attempts_left } => { print!("UV failed."); if let Some(attempts_left) = attempts_left { diff --git a/libwebauthn/examples/webauthn_cable.rs b/libwebauthn/examples/webauthn_cable.rs index 389a43f8..b6f4c7ba 100644 --- a/libwebauthn/examples/webauthn_cable.rs +++ b/libwebauthn/examples/webauthn_cable.rs @@ -44,6 +44,8 @@ async fn handle_updates(mut state_recv: Receiver) { match update { CableUxUpdate::UvUpdate(uv_update) => match uv_update { UvUpdate::PresenceRequired => println!("Please touch your device!"), + // Can't happen, as we are using cable that doesn't do PIN + UvUpdate::PinNotSet(_) => panic!("Pin not set error for cable!"), UvUpdate::UvRetry { attempts_left } => { print!("UV failed."); if let Some(attempts_left) = attempts_left { diff --git a/libwebauthn/examples/webauthn_extensions_hid.rs b/libwebauthn/examples/webauthn_extensions_hid.rs index 63ffdb80..321ed311 100644 --- a/libwebauthn/examples/webauthn_extensions_hid.rs +++ b/libwebauthn/examples/webauthn_extensions_hid.rs @@ -36,6 +36,8 @@ async fn handle_updates(mut state_recv: Receiver) { while let Ok(update) = state_recv.recv().await { match update { UvUpdate::PresenceRequired => println!("Please touch your device!"), + // Shouldn't really happen, as we don't use UV required + UvUpdate::PinNotSet(_) => println!("Pin not set for your device!"), UvUpdate::UvRetry { attempts_left } => { print!("UV failed."); if let Some(attempts_left) = attempts_left { diff --git a/libwebauthn/examples/webauthn_hid.rs b/libwebauthn/examples/webauthn_hid.rs index 982042db..57ed0428 100644 --- a/libwebauthn/examples/webauthn_hid.rs +++ b/libwebauthn/examples/webauthn_hid.rs @@ -13,7 +13,7 @@ use libwebauthn::ops::webauthn::{ GetAssertionRequest, GetAssertionRequestExtensions, MakeCredentialRequest, ResidentKeyRequirement, UserVerificationRequirement, }; -use libwebauthn::pin::PinRequestReason; +use libwebauthn::pin::{PinNotSetReason, PinRequestReason}; use libwebauthn::proto::ctap2::{ Ctap2CredentialType, Ctap2PublicKeyCredentialDescriptor, Ctap2PublicKeyCredentialRpEntity, Ctap2PublicKeyCredentialUserEntity, @@ -35,6 +35,25 @@ async fn handle_updates(mut state_recv: Receiver) { while let Ok(update) = state_recv.recv().await { match update { UvUpdate::PresenceRequired => println!("Please touch your device!"), + UvUpdate::PinNotSet(update) => { + match update.reason { + PinNotSetReason::PinNotSet => println!("RP required a PIN, but your device has none set. Please set one now (this is NOT a RP-specific operation, but will require a PIN on your device from now on, if you continue. Leave the pini entry empty to cancel the operation.)"), + PinNotSetReason::PinTooShort => println!("The provided PIN was too short"), + PinNotSetReason::PinTooLong => println!("The provided PIN was too long"), + PinNotSetReason::PinPolicyViolation => println!("The provided PIN violated the pin policy set on the device."), + } + print!("PIN: Please set a new PIN for your device: "); + io::stdout().flush().unwrap(); + let pin_raw: String = read!("{}\n"); + + if pin_raw.is_empty() { + println!("PIN: No PIN provided, cancelling operation."); + update.cancel(); + } else { + let _ = update.set_pin(&pin_raw); + println!(); + } + } UvUpdate::UvRetry { attempts_left } => { print!("UV failed."); if let Some(attempts_left) = attempts_left { @@ -86,6 +105,18 @@ pub async fn main() -> Result<(), Box> { let mut channel = device.channel().await?; channel.wink(TIMEOUT).await?; + // Ask user what kind of request should be issued + let user_verification = { + print!("Do you want to require user verification in the request? [y/N]: "); + io::stdout().flush().expect("Failed to flush stdout!"); + let input: String = read!("{}\n"); + if input.trim() == "y" || input.trim() == "Y" { + UserVerificationRequirement::Required + } else { + UserVerificationRequirement::Preferred + } + }; + // Make Credentials ceremony let make_credentials_request = MakeCredentialRequest { challenge: Vec::from(challenge), @@ -94,7 +125,7 @@ pub async fn main() -> Result<(), Box> { relying_party: Ctap2PublicKeyCredentialRpEntity::new("example.org", "example.org"), user: Ctap2PublicKeyCredentialUserEntity::new(&user_id, "mario.rossi", "Mario Rossi"), resident_key: Some(ResidentKeyRequirement::Discouraged), - user_verification: UserVerificationRequirement::Preferred, + user_verification, algorithms: vec![Ctap2CredentialType::default()], exclude: None, extensions: None, diff --git a/libwebauthn/examples/webauthn_json_hid.rs b/libwebauthn/examples/webauthn_json_hid.rs index 1e319207..3101a3b2 100644 --- a/libwebauthn/examples/webauthn_json_hid.rs +++ b/libwebauthn/examples/webauthn_json_hid.rs @@ -29,6 +29,8 @@ async fn handle_updates(mut state_recv: Receiver) { while let Ok(update) = state_recv.recv().await { match update { UvUpdate::PresenceRequired => println!("Please touch your device!"), + // Shouldn't really happen, as we don't use UV required + UvUpdate::PinNotSet(_) => println!("Pin not set for your device!"), UvUpdate::UvRetry { attempts_left } => { print!("UV failed."); if let Some(attempts_left) = attempts_left { diff --git a/libwebauthn/examples/webauthn_nfc.rs b/libwebauthn/examples/webauthn_nfc.rs index 59b8e53f..62eec065 100644 --- a/libwebauthn/examples/webauthn_nfc.rs +++ b/libwebauthn/examples/webauthn_nfc.rs @@ -11,7 +11,7 @@ use tracing_subscriber::{self, EnvFilter}; use libwebauthn::ops::webauthn::{ GetAssertionRequest, MakeCredentialRequest, ResidentKeyRequirement, UserVerificationRequirement, }; -use libwebauthn::pin::PinRequestReason; +use libwebauthn::pin::{PinNotSetReason, PinRequestReason}; use libwebauthn::proto::ctap2::{ Ctap2CredentialType, Ctap2PublicKeyCredentialDescriptor, Ctap2PublicKeyCredentialRpEntity, Ctap2PublicKeyCredentialUserEntity, @@ -33,6 +33,25 @@ async fn handle_updates(mut state_recv: Receiver) { while let Ok(update) = state_recv.recv().await { match update { UvUpdate::PresenceRequired => println!("Please touch your device!"), + UvUpdate::PinNotSet(update) => { + match update.reason { + PinNotSetReason::PinNotSet => println!("RP required a PIN, but your device has none set. Please set one now (this is NOT a RP-specific operation, but will require a PIN on your device from now on, if you continue. Leave the pini entry empty to cancel the operation.)"), + PinNotSetReason::PinTooShort => println!("The provided PIN was too short"), + PinNotSetReason::PinTooLong => println!("The provided PIN was too long"), + PinNotSetReason::PinPolicyViolation => println!("The provided PIN violated the pin policy set on the device."), + } + print!("PIN: Please set a new PIN for your device: "); + io::stdout().flush().unwrap(); + let pin_raw: String = read!("{}\n"); + + if pin_raw.is_empty() { + println!("PIN: No PIN provided, cancelling operation."); + update.cancel(); + } else { + let _ = update.set_pin(&pin_raw); + println!(); + } + } UvUpdate::UvRetry { attempts_left } => { print!("UV failed."); if let Some(attempts_left) = attempts_left { @@ -87,6 +106,18 @@ pub async fn main() -> Result<(), Box> { println!("Selected NFC authenticator: {}", device); let mut channel = device.channel().await?; + // Ask user what kind of request should be issued + let user_verification = { + print!("Do you want to require user verification in the request? [y/N]: "); + io::stdout().flush().expect("Failed to flush stdout!"); + let input: String = read!("{}\n"); + if input.trim() == "y" || input.trim() == "Y" { + UserVerificationRequirement::Required + } else { + UserVerificationRequirement::Preferred + } + }; + // Make Credentials ceremony let make_credentials_request = MakeCredentialRequest { challenge: Vec::from(challenge), @@ -95,7 +126,7 @@ pub async fn main() -> Result<(), Box> { relying_party: Ctap2PublicKeyCredentialRpEntity::new("example.org", "example.org"), user: Ctap2PublicKeyCredentialUserEntity::new(&user_id, "mario.rossi", "Mario Rossi"), resident_key: Some(ResidentKeyRequirement::Discouraged), - user_verification: UserVerificationRequirement::Preferred, + user_verification, algorithms: vec![Ctap2CredentialType::default()], exclude: None, extensions: None, diff --git a/libwebauthn/examples/webauthn_preflight_hid.rs b/libwebauthn/examples/webauthn_preflight_hid.rs index abb4751a..8481291a 100644 --- a/libwebauthn/examples/webauthn_preflight_hid.rs +++ b/libwebauthn/examples/webauthn_preflight_hid.rs @@ -35,6 +35,8 @@ async fn handle_updates(mut state_recv: Receiver) { while let Ok(update) = state_recv.recv().await { match update { UvUpdate::PresenceRequired => println!("Please touch your device!"), + // Shouldn't really happen, as we don't use UV required + UvUpdate::PinNotSet(_) => println!("Pin not set for your device!"), UvUpdate::UvRetry { attempts_left } => { print!("UV failed."); if let Some(attempts_left) = attempts_left { diff --git a/libwebauthn/examples/webauthn_prf_hid.rs b/libwebauthn/examples/webauthn_prf_hid.rs index 96605cfa..167b557d 100644 --- a/libwebauthn/examples/webauthn_prf_hid.rs +++ b/libwebauthn/examples/webauthn_prf_hid.rs @@ -38,6 +38,8 @@ async fn handle_updates(mut state_recv: Receiver) { while let Ok(update) = state_recv.recv().await { match update { UvUpdate::PresenceRequired => println!("Please touch your device!"), + // Shouldn't really happen, as we don't use UV required + UvUpdate::PinNotSet(_) => println!("Pin not set for your device!"), UvUpdate::UvRetry { attempts_left } => { print!("UV failed."); if let Some(attempts_left) = attempts_left { diff --git a/libwebauthn/src/lib.rs b/libwebauthn/src/lib.rs index f66f5993..8efcff0f 100644 --- a/libwebauthn/src/lib.rs +++ b/libwebauthn/src/lib.rs @@ -40,7 +40,7 @@ macro_rules! unwrap_field { } }}; } -use pin::PinRequestReason; +use pin::{PinNotSetReason, PinRequestReason}; pub(crate) use unwrap_field; #[derive(Debug)] @@ -61,6 +61,7 @@ pub enum UvUpdate { /// The ongoing operation may run into a timeout, no answer is provided in time. PinRequired(PinRequiredUpdate), PresenceRequired, + PinNotSet(PinNotSetUpdate), } #[derive(Debug, Clone)] @@ -90,6 +91,31 @@ impl PinRequiredUpdate { } } +#[derive(Debug, Clone)] +pub struct PinNotSetUpdate { + reply_to: Arc>, + /// What caused the PIN request. + pub reason: PinNotSetReason, +} + +impl PinNotSetUpdate { + /// This consumes `self`, because we should only ever send exactly one answer back. + pub fn set_pin(self, pin: &str) -> Result<(), String> { + match Arc::into_inner(self.reply_to) { + Some(sender) => sender + .send(pin.to_string()) + .map_err(|_| "Failed to send PIN".to_string()), + None => Err("Multiple references to reply_to exist; cannot send PIN".to_string()), + } + } + + /// The user cancels the PIN entry, without making an attempt. + pub fn cancel(self) { + // We hang up to signal an abort + drop(self.reply_to) + } +} + #[cfg(test)] // This function is not _really_ `PartialEq`. We need it for testing purposes, // but should not expose it like this to consumers, so gating it behind cfg(test) @@ -100,6 +126,16 @@ impl PartialEq for PinRequiredUpdate { } } +#[cfg(test)] +// This function is not _really_ `PartialEq`. We need it for testing purposes, +// but should not expose it like this to consumers, so gating it behind cfg(test) +impl PartialEq for PinNotSetUpdate { + fn eq(&self, other: &Self) -> bool { + // We explicitly ignore `reply_to` and only compare the other fields. + self.reason == other.reason + } +} + pub fn available_transports() -> Vec { vec![Transport::Usb, Transport::Ble] } diff --git a/libwebauthn/src/pin.rs b/libwebauthn/src/pin.rs index 7414f282..c164ba6a 100644 --- a/libwebauthn/src/pin.rs +++ b/libwebauthn/src/pin.rs @@ -17,7 +17,7 @@ use x509_parser::nom::AsBytes; use crate::{ proto::{ - ctap2::{Ctap2, Ctap2ClientPinRequest, Ctap2PinUvAuthProtocol}, + ctap2::{Ctap2, Ctap2ClientPinRequest, Ctap2GetInfoResponse, Ctap2PinUvAuthProtocol}, CtapError, }, transport::Channel, @@ -49,6 +49,18 @@ pub enum PinRequestReason { // Passkey } +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +pub enum PinNotSetReason { + /// Device does not have a PIN set, but needs one + PinNotSet, + /// PIN too short + PinTooShort, + /// PIN too long + PinTooLong, + /// PIN violates PinPolicy + PinPolicyViolation, +} + pub trait PinUvAuthProtocol: Send + Sync { fn version(&self) -> Ctap2PinUvAuthProtocol; @@ -422,7 +434,18 @@ pub fn hkdf_sha256(salt: Option<&[u8]>, ikm: &[u8], info: &[u8]) -> Result Result<(), Error>; +} + +#[async_trait] +#[allow(private_bounds)] +pub trait PinManagement: PinManagementInternal { async fn change_pin(&mut self, new_pin: String, timeout: Duration) -> Result<(), Error>; } @@ -433,7 +456,22 @@ where { async fn change_pin(&mut self, new_pin: String, timeout: Duration) -> Result<(), Error> { let get_info_response = self.ctap2_get_info().await?; + self.change_pin_internal(&get_info_response, new_pin, timeout) + .await + } +} +#[async_trait] +impl PinManagementInternal for C +where + C: Channel, +{ + async fn change_pin_internal( + &mut self, + get_info_response: &Ctap2GetInfoResponse, + new_pin: String, + timeout: Duration, + ) -> Result<(), Error> { // If the minPINLength member of the authenticatorGetInfo response is absent, then let platformMinPINLengthInCodePoints be 4. if new_pin.len() < get_info_response.min_pin_length.unwrap_or(4) as usize { // If platformCollectedPinLengthInCodePoints is less than platformMinPINLengthInCodePoints then the platform SHOULD display a "PIN too short" error message to the user. @@ -448,7 +486,7 @@ where let Some(uv_proto) = select_uv_proto( #[cfg(test)] self.get_forced_pin_protocol(), - &get_info_response, + get_info_response, ) .await else { @@ -466,7 +504,7 @@ where Some(true) => Some( obtain_pin( self, - &get_info_response, + get_info_response, uv_proto.version(), PinRequestReason::AuthenticatorPolicy, timeout, diff --git a/libwebauthn/src/webauthn/pin_uv_auth_token.rs b/libwebauthn/src/webauthn/pin_uv_auth_token.rs index 652d2553..7d78e1a1 100644 --- a/libwebauthn/src/webauthn/pin_uv_auth_token.rs +++ b/libwebauthn/src/webauthn/pin_uv_auth_token.rs @@ -7,7 +7,8 @@ use cosey::PublicKey; use crate::ops::webauthn::UserVerificationRequirement; use crate::pin::{ - pin_hash, PinRequestReason, PinUvAuthProtocol, PinUvAuthProtocolOne, PinUvAuthProtocolTwo, + pin_hash, PinManagementInternal, PinNotSetReason, PinRequestReason, PinUvAuthProtocol, + PinUvAuthProtocolOne, PinUvAuthProtocolTwo, }; use crate::proto::ctap2::{ Ctap2, Ctap2ClientPinRequest, Ctap2GetInfoResponse, Ctap2PinUvAuthProtocol, @@ -16,7 +17,7 @@ use crate::proto::ctap2::{ pub use crate::transport::error::TransportError; use crate::transport::{AuthTokenData, Channel, Ctap2AuthTokenPermission}; pub use crate::webauthn::error::{CtapError, Error, PlatformError}; -use crate::{PinRequiredUpdate, UvUpdate}; +use crate::{PinNotSetUpdate, PinRequiredUpdate, UvUpdate}; #[derive(Debug, Copy, Clone, PartialEq, Eq)] @@ -59,7 +60,7 @@ where C: Channel, R: Ctap2UserVerifiableRequest, { - let get_info_response = channel.ctap2_get_info().await?; + let mut get_info_response = channel.ctap2_get_info().await?; ctap2_request.handle_legacy_preview(&get_info_response); let maybe_uv_proto = select_uv_proto( #[cfg(test)] @@ -81,7 +82,7 @@ where user_verification_helper( channel, - &get_info_response, + &mut get_info_response, user_verification, ctap2_request, timeout, @@ -92,7 +93,7 @@ where #[instrument(skip_all)] async fn user_verification_helper( channel: &mut C, - get_info_response: &Ctap2GetInfoResponse, + get_info_response: &mut Ctap2GetInfoResponse, user_verification: UserVerificationRequirement, ctap2_request: &mut R, timeout: Duration, @@ -117,16 +118,29 @@ where return Ok(UsedPinUvAuthToken::None); } - if !dev_uv_protected && user_verification.is_required() { - error!( - "Request requires user verification, but device user verification is not available." - ); - return Err(Error::Ctap(CtapError::PINNotSet)); - }; - - if !dev_uv_protected && user_verification.is_preferred() { - warn!("User verification is preferred, but device user verification is not available. Ignoring."); - return Ok(UsedPinUvAuthToken::None); + if !dev_uv_protected { + if user_verification.is_required() { + error!( + "Request requires user verification, but device user verification is not available. Try letting the user set a PIN." + ); + // Lets try setting a PIN. Either we succeed in some fashion, or we return the resulting error here instead. + try_to_set_pin( + channel, + get_info_response, + PinNotSetReason::PinNotSet, + timeout, + ) + .await?; + // Update get_info_response, because now maybe "clientPin" is set to `Some(true)` + *get_info_response = channel.ctap2_get_info().await?; + // Then simply continue with the normal flow. Either we have a PIN set now, + // then the user has to enter it (again), as in the normal flow, or the device + // itself has some kind of internal protocol to handle this situation. + // If not, it will (should) return an error like PinNotSet. + } else if user_verification.is_preferred() { + warn!("User verification is preferred, but device user verification is not available. Ignoring."); + return Ok(UsedPinUvAuthToken::None); + } } } else if !can_establish_shared_secret && !uv { // We need a shared secret, but the device does not support any form of query-able UV, so we can't establish a @@ -441,27 +455,92 @@ where Ok(pin.as_bytes().to_owned()) } +pub(crate) async fn try_to_set_pin( + channel: &mut C, + info: &Ctap2GetInfoResponse, + mut reason: PinNotSetReason, + timeout: Duration, +) -> Result<(), Error> +where + C: Channel, +{ + // If we have any Ctap2GetInfoResponse, we are already doing FIDO2, + // so setting PIN is theoretically possible + + // Does the device even support a PIN? + if !info.option_exists("clientPin") { + // It does not. Maybe it has an built in internal way of handling this. + // Lets continue and see what happens. + return Ok(()); + } + + // Try to get a suitable PIN from the user until it meets the criteria + loop { + // Obtain PIN from user + let (tx, rx) = tokio::sync::oneshot::channel(); + channel + .send_ux_update( + UvUpdate::PinNotSet(PinNotSetUpdate { + reply_to: Arc::new(tx), + reason, + }) + .into(), + ) + .await; + let pin = match rx.await { + Ok(pin) => pin, + Err(_) => { + info!("User cancelled operation: no PIN provided"); + return Err(Error::Platform(PlatformError::Cancelled)); + } + }; + match channel + .change_pin_internal(info, pin.clone(), timeout) + .await + { + Ok(()) => { + // PIN was successfully set. The user can now try to finish the ongoing operation. + return Ok(()); + } + Err(Error::Platform(PlatformError::PinTooShort)) => { + reason = PinNotSetReason::PinTooShort; + continue; + } + Err(Error::Platform(PlatformError::PinTooLong)) => { + reason = PinNotSetReason::PinTooLong; + continue; + } + Err(Error::Ctap(CtapError::PINPolicyViolation)) => { + reason = PinNotSetReason::PinPolicyViolation; + continue; + } + // General, not PIN-related error + Err(err) => { + return Err(err); + } + } + } +} + #[cfg(test)] mod test { use std::{collections::HashMap, time::Duration}; use serde_bytes::ByteBuf; + use tokio::sync::broadcast::Receiver; use crate::{ ops::webauthn::{ GetAssertionRequest, GetAssertionRequestExtensions, PRFValue, PrfInput, UserVerificationRequirement, }, - pin::{pin_hash, PinUvAuthProtocol, PinUvAuthProtocolOne}, - proto::{ - ctap2::{ - cbor::{to_vec, CborRequest, CborResponse}, - Ctap2ClientPinRequest, Ctap2ClientPinResponse, Ctap2CommandCode, - Ctap2GetAssertionRequest, Ctap2GetInfoResponse, Ctap2PinUvAuthProtocol, - Ctap2UserVerifiableRequest, Ctap2UserVerificationOperation, - }, - CtapError, + pin::{pin_hash, PinNotSetReason, PinUvAuthProtocol, PinUvAuthProtocolOne}, + proto::ctap2::{ + cbor::{to_vec, CborRequest, CborResponse}, + Ctap2ClientPinRequest, Ctap2ClientPinResponse, Ctap2CommandCode, + Ctap2GetAssertionRequest, Ctap2GetInfoResponse, Ctap2PinUvAuthProtocol, + Ctap2UserVerifiableRequest, Ctap2UserVerificationOperation, }, transport::{mock::channel::MockChannel, Channel, Ctap2AuthTokenStore}, webauthn::UsedPinUvAuthToken, @@ -539,6 +618,68 @@ mod test { assert!(status_recv.is_empty()); } + async fn handle_setting_pin_updates( + mut state_recv: Receiver, + expected_reasons: Vec, + pin_answers: Vec, + ) -> () { + let mut idx = 0; + loop { + let update = state_recv + .recv() + .await + .expect("Failed to receive UV update"); + match update { + UvUpdate::PinNotSet(pinnotset) => { + assert_eq!(pinnotset.reason, expected_reasons[idx]); + if idx >= pin_answers.len() { + break; + } + pinnotset.set_pin(&pin_answers[idx]).unwrap(); + } + e => { + panic!("Received unexpected UvUpdate: {e:?}"); + } + } + idx += 1; + } + // Drop state_recv here, to cancel operation + } + + async fn test_setting_pin(expected_reasons: Vec, pin_answers: Vec) { + let mut channel = MockChannel::new(); + let status_recv = channel.get_ux_update_receiver(); + let info = create_info(&[("clientPin", false)], None); + let info_req = CborRequest::new(Ctap2CommandCode::AuthenticatorGetInfo); + let info_resp = CborResponse::new_success_from_slice(to_vec(&info).unwrap().as_slice()); + + let handle = tokio::task::spawn(handle_setting_pin_updates( + status_recv, + expected_reasons, + pin_answers, + )); + + channel.push_command_pair(info_req, info_resp); + + let mut getassertion = create_get_assertion(&info, None); + + // We should early return here right at the start and not send a ClientPIN-request + let resp = user_verification( + &mut channel, + UserVerificationRequirement::Required, + &mut getassertion, + TIMEOUT, + ) + .await; + + handle.await.unwrap(); + + assert_eq!( + resp, + Err(Error::Platform(crate::webauthn::PlatformError::Cancelled)) + ); + } + #[tokio::test] async fn early_exit_device_no_options() { test_early_exits( @@ -576,22 +717,21 @@ mod test { } #[tokio::test] - async fn early_exit_device_client_pin_not_set_but_uv_required() { - let testcases = vec![ - vec![], - // Should be the same as above - vec![("clientPin", false)], + async fn device_client_pin_not_set_but_uv_required_hanging_up() { + let expected_reasons = vec![PinNotSetReason::PinNotSet]; + let pin_answers = vec![]; + test_setting_pin(expected_reasons, pin_answers).await; + } + + #[tokio::test] + async fn device_client_pin_not_set_but_uv_required_try_setting_pin() { + let expected_reasons = vec![ + PinNotSetReason::PinNotSet, + PinNotSetReason::PinTooShort, + PinNotSetReason::PinTooLong, ]; - for testcase in testcases { - test_early_exits( - &testcase, - None, - UserVerificationRequirement::Required, - None, - Err(Error::Ctap(CtapError::PINNotSet)), - ) - .await; - } + let pin_answers = vec![String::from("1"), "1".repeat(1000)]; + test_setting_pin(expected_reasons, pin_answers).await; } #[tokio::test] From 167296deb1cf649762df16fabecfa66056a7b1f3 Mon Sep 17 00:00:00 2001 From: Martin Sirringhaus Date: Tue, 10 Mar 2026 14:03:07 +0100 Subject: [PATCH 2/2] Address review feedback: Use sealed trait and add positive test case --- libwebauthn/src/pin.rs | 247 +++++++++--------- libwebauthn/src/transport/mock/channel.rs | 2 +- libwebauthn/src/webauthn/pin_uv_auth_token.rs | 175 ++++++++++++- 3 files changed, 300 insertions(+), 124 deletions(-) diff --git a/libwebauthn/src/pin.rs b/libwebauthn/src/pin.rs index c164ba6a..737b6c9e 100644 --- a/libwebauthn/src/pin.rs +++ b/libwebauthn/src/pin.rs @@ -433,18 +433,136 @@ pub fn hkdf_sha256(salt: Option<&[u8]>, ikm: &[u8], info: &[u8]) -> Result Result<(), Error>; +// Defining a sealed trait for the internal API that is only visible within the crate +// while not touching/extending the external API +pub(crate) mod internal { + use super::*; + + #[async_trait] + pub trait PinManagementInternal { + async fn change_pin_internal( + &mut self, + get_info_response: &Ctap2GetInfoResponse, + new_pin: String, + timeout: Duration, + ) -> Result<(), Error>; + } + + #[async_trait] + impl PinManagementInternal for C + where + C: Channel, + { + async fn change_pin_internal( + &mut self, + get_info_response: &Ctap2GetInfoResponse, + new_pin: String, + timeout: Duration, + ) -> Result<(), Error> { + // If the minPINLength member of the authenticatorGetInfo response is absent, then let platformMinPINLengthInCodePoints be 4. + if new_pin.len() < get_info_response.min_pin_length.unwrap_or(4) as usize { + // If platformCollectedPinLengthInCodePoints is less than platformMinPINLengthInCodePoints then the platform SHOULD display a "PIN too short" error message to the user. + return Err(Error::Platform(PlatformError::PinTooShort)); + } + + // If the byte length of "newPin" is greater than the max UTF-8 representation limit of 63 bytes, then the platform SHOULD display a "PIN too long" error message to the user. + if new_pin.len() >= 64 { + return Err(Error::Platform(PlatformError::PinTooLong)); + } + + let Some(uv_proto) = select_uv_proto( + #[cfg(test)] + self.get_forced_pin_protocol(), + get_info_response, + ) + .await + else { + error!("No supported PIN/UV auth protocols found"); + return Err(Error::Ctap(CtapError::Other)); + }; + + let current_pin = match get_info_response + .options + .as_ref() + .ok_or(Error::Platform(PlatformError::InvalidDeviceResponse))? + .get("clientPin") + { + // Obtaining the current PIN, if one is set + Some(true) => Some( + obtain_pin( + self, + get_info_response, + uv_proto.version(), + PinRequestReason::AuthenticatorPolicy, + timeout, + ) + .await?, + ), + + // No PIN set yet + Some(false) => None, + + // Device does not support PIN + None => { + return Err(Error::Platform(PlatformError::PinNotSupported)); + } + }; + + // In preparation for obtaining pinUvAuthToken, the platform: + // * Obtains a shared secret. + let (public_key, shared_secret) = + obtain_shared_secret(self, uv_proto.as_ref(), timeout).await?; + + // paddedPin is newPin padded on the right with 0x00 bytes to make it 64 bytes long. (Since the maximum length of newPin is 63 bytes, there is always at least one byte of padding.) + let mut padded_new_pin = new_pin.as_bytes().to_vec(); + padded_new_pin.resize(64, 0x00); + + // newPinEnc: the result of calling encrypt(shared secret, paddedPin) where + let new_pin_enc = uv_proto.encrypt(&shared_secret, &padded_new_pin)?; + + let req = match current_pin { + Some(curr_pin) => { + // pinHashEnc: The result of calling encrypt(shared secret, LEFT(SHA-256(curPin), 16)). + let pin_hash = pin_hash(&curr_pin); + let pin_hash_enc = uv_proto.encrypt(&shared_secret, &pin_hash)?; + + // pinUvAuthParam: the result of calling authenticate(shared secret, newPinEnc || pinHashEnc) + let uv_auth_param = uv_proto.authenticate( + &shared_secret, + &[new_pin_enc.as_slice(), pin_hash_enc.as_slice()].concat(), + )?; + + Ctap2ClientPinRequest::new_change_pin( + uv_proto.version(), + &new_pin_enc, + &pin_hash_enc, + public_key, + &uv_auth_param, + ) + } + None => { + // pinUvAuthParam: the result of calling authenticate(shared secret, newPinEnc). + let uv_auth_param = uv_proto.authenticate(&shared_secret, &new_pin_enc)?; + + Ctap2ClientPinRequest::new_set_pin( + uv_proto.version(), + &new_pin_enc, + public_key, + &uv_auth_param, + ) + } + }; + + // On success, this is an all-empty Ctap2ClientPinResponse + let _ = self.ctap2_client_pin(&req, timeout).await?; + Ok(()) + } + } } +use internal::PinManagementInternal; + #[async_trait] -#[allow(private_bounds)] pub trait PinManagement: PinManagementInternal { async fn change_pin(&mut self, new_pin: String, timeout: Duration) -> Result<(), Error>; } @@ -460,114 +578,3 @@ where .await } } - -#[async_trait] -impl PinManagementInternal for C -where - C: Channel, -{ - async fn change_pin_internal( - &mut self, - get_info_response: &Ctap2GetInfoResponse, - new_pin: String, - timeout: Duration, - ) -> Result<(), Error> { - // If the minPINLength member of the authenticatorGetInfo response is absent, then let platformMinPINLengthInCodePoints be 4. - if new_pin.len() < get_info_response.min_pin_length.unwrap_or(4) as usize { - // If platformCollectedPinLengthInCodePoints is less than platformMinPINLengthInCodePoints then the platform SHOULD display a "PIN too short" error message to the user. - return Err(Error::Platform(PlatformError::PinTooShort)); - } - - // If the byte length of "newPin" is greater than the max UTF-8 representation limit of 63 bytes, then the platform SHOULD display a "PIN too long" error message to the user. - if new_pin.len() >= 64 { - return Err(Error::Platform(PlatformError::PinTooLong)); - } - - let Some(uv_proto) = select_uv_proto( - #[cfg(test)] - self.get_forced_pin_protocol(), - get_info_response, - ) - .await - else { - error!("No supported PIN/UV auth protocols found"); - return Err(Error::Ctap(CtapError::Other)); - }; - - let current_pin = match get_info_response - .options - .as_ref() - .ok_or(Error::Platform(PlatformError::InvalidDeviceResponse))? - .get("clientPin") - { - // Obtaining the current PIN, if one is set - Some(true) => Some( - obtain_pin( - self, - get_info_response, - uv_proto.version(), - PinRequestReason::AuthenticatorPolicy, - timeout, - ) - .await?, - ), - - // No PIN set yet - Some(false) => None, - - // Device does not support PIN - None => { - return Err(Error::Platform(PlatformError::PinNotSupported)); - } - }; - - // In preparation for obtaining pinUvAuthToken, the platform: - // * Obtains a shared secret. - let (public_key, shared_secret) = - obtain_shared_secret(self, uv_proto.as_ref(), timeout).await?; - - // paddedPin is newPin padded on the right with 0x00 bytes to make it 64 bytes long. (Since the maximum length of newPin is 63 bytes, there is always at least one byte of padding.) - let mut padded_new_pin = new_pin.as_bytes().to_vec(); - padded_new_pin.resize(64, 0x00); - - // newPinEnc: the result of calling encrypt(shared secret, paddedPin) where - let new_pin_enc = uv_proto.encrypt(&shared_secret, &padded_new_pin)?; - - let req = match current_pin { - Some(curr_pin) => { - // pinHashEnc: The result of calling encrypt(shared secret, LEFT(SHA-256(curPin), 16)). - let pin_hash = pin_hash(&curr_pin); - let pin_hash_enc = uv_proto.encrypt(&shared_secret, &pin_hash)?; - - // pinUvAuthParam: the result of calling authenticate(shared secret, newPinEnc || pinHashEnc) - let uv_auth_param = uv_proto.authenticate( - &shared_secret, - &[new_pin_enc.as_slice(), pin_hash_enc.as_slice()].concat(), - )?; - - Ctap2ClientPinRequest::new_change_pin( - uv_proto.version(), - &new_pin_enc, - &pin_hash_enc, - public_key, - &uv_auth_param, - ) - } - None => { - // pinUvAuthParam: the result of calling authenticate(shared secret, newPinEnc). - let uv_auth_param = uv_proto.authenticate(&shared_secret, &new_pin_enc)?; - - Ctap2ClientPinRequest::new_set_pin( - uv_proto.version(), - &new_pin_enc, - public_key, - &uv_auth_param, - ) - } - }; - - // On success, this is an all-empty Ctap2ClientPinResponse - let _ = self.ctap2_client_pin(&req, timeout).await?; - Ok(()) - } -} diff --git a/libwebauthn/src/transport/mock/channel.rs b/libwebauthn/src/transport/mock/channel.rs index 43936bc8..0c9f480c 100644 --- a/libwebauthn/src/transport/mock/channel.rs +++ b/libwebauthn/src/transport/mock/channel.rs @@ -100,7 +100,7 @@ impl Channel for MockChannel { assert_eq!( &expected, request, - "{} items still in the queue", + "{} items still in the queue. Left: Expected, Right: Received", self.expected_requests.len() ); Ok(()) diff --git a/libwebauthn/src/webauthn/pin_uv_auth_token.rs b/libwebauthn/src/webauthn/pin_uv_auth_token.rs index 7d78e1a1..18ce7393 100644 --- a/libwebauthn/src/webauthn/pin_uv_auth_token.rs +++ b/libwebauthn/src/webauthn/pin_uv_auth_token.rs @@ -7,8 +7,8 @@ use cosey::PublicKey; use crate::ops::webauthn::UserVerificationRequirement; use crate::pin::{ - pin_hash, PinManagementInternal, PinNotSetReason, PinRequestReason, PinUvAuthProtocol, - PinUvAuthProtocolOne, PinUvAuthProtocolTwo, + internal::PinManagementInternal, pin_hash, PinNotSetReason, PinRequestReason, + PinUvAuthProtocol, PinUvAuthProtocolOne, PinUvAuthProtocolTwo, }; use crate::proto::ctap2::{ Ctap2, Ctap2ClientPinRequest, Ctap2GetInfoResponse, Ctap2PinUvAuthProtocol, @@ -663,7 +663,7 @@ mod test { let mut getassertion = create_get_assertion(&info, None); - // We should early return here right at the start and not send a ClientPIN-request + // We should receive a PinNotSet-UvUpdate here before a clientPIN-request is issued let resp = user_verification( &mut channel, UserVerificationRequirement::Required, @@ -734,6 +734,175 @@ mod test { test_setting_pin(expected_reasons, pin_answers).await; } + #[tokio::test] + async fn device_client_pin_not_set_but_uv_required_good_path() { + // Mostly copy&paste from full_ceremony_using_pin() testcase, with + // setting PIN wedged inbetween + let mut channel = MockChannel::new(); + let mut info = create_info(&[("clientPin", false), ("pinUvAuthToken", true)], None); + info.pin_auth_protos = Some(vec![1]); + + // Instrumenting the MockChannel with expected requests and responses + // + // 1. GetInfo request + let info_req = CborRequest::new(Ctap2CommandCode::AuthenticatorGetInfo); + let info_resp = CborResponse::new_success_from_slice(to_vec(&info).unwrap().as_slice()); + channel.push_command_pair(info_req, info_resp); + + // 2. KeyAgreement request and response (happens earlier than in full_ceremony_using_pin(), because we need it for setting the PIN, too) + let key_agreement_req = CborRequest::try_from( + &Ctap2ClientPinRequest::new_get_key_agreement(Ctap2PinUvAuthProtocol::One), + ) + .unwrap(); + let key_agreement = get_key_agreement(); + let key_agreement_resp = CborResponse::new_success_from_slice( + to_vec(&Ctap2ClientPinResponse { + key_agreement: Some(key_agreement.clone()), + pin_uv_auth_token: None, + pin_retries: None, + power_cycle_state: None, + uv_retries: None, + }) + .unwrap() + .as_slice(), + ); + channel.push_command_pair(key_agreement_req.clone(), key_agreement_resp.clone()); + + // 3. ClientPin request (set PIN) + let pin_protocol = PinUvAuthProtocolOne::new(); + let (public_key, shared_secret) = pin_protocol.encapsulate(&get_key_agreement()).unwrap(); + let mut padded_new_pin = "1234".as_bytes().to_vec(); + padded_new_pin.resize(64, 0x00); + let new_pin_enc = pin_protocol + .encrypt(&shared_secret, &padded_new_pin) + .unwrap(); + let uv_auth_param = pin_protocol + .authenticate(&shared_secret, &new_pin_enc) + .unwrap(); + let set_pin_req = CborRequest::try_from(&Ctap2ClientPinRequest::new_set_pin( + pin_protocol.version(), + &new_pin_enc, + public_key.clone(), + &uv_auth_param, + )) + .unwrap(); + + let set_pin_resp = CborResponse::new_success_from_slice( + to_vec(&Ctap2ClientPinResponse::default()) // all None + .unwrap() + .as_slice(), + ); + channel.push_command_pair(set_pin_req, set_pin_resp); + + let mut info = create_info(&[("clientPin", true), ("pinUvAuthToken", true)], None); + info.pin_auth_protos = Some(vec![1]); + let mut getassertion = create_get_assertion(&info, None); + + // 4. Second GetInfo request, this time with clientPin set to true + let info_req = CborRequest::new(Ctap2CommandCode::AuthenticatorGetInfo); + let info_resp = CborResponse::new_success_from_slice(to_vec(&info).unwrap().as_slice()); + channel.push_command_pair(info_req, info_resp); + + // 5. Queueing PinRetries request and response + let pin_retries_req = CborRequest::try_from(&Ctap2ClientPinRequest::new_get_pin_retries( + Some(Ctap2PinUvAuthProtocol::One), + )) + .unwrap(); + let pin_retries_resp = CborResponse::new_success_from_slice( + to_vec(&Ctap2ClientPinResponse { + key_agreement: None, + pin_uv_auth_token: None, + pin_retries: Some(5), + power_cycle_state: None, + uv_retries: None, + }) + .unwrap() + .as_slice(), + ); + channel.push_command_pair(pin_retries_req, pin_retries_resp); + + // 6. Second key agreement call (reusing the previous one) + channel.push_command_pair(key_agreement_req, key_agreement_resp); + + // 7. getPinUvAuth request and response + let pin_hash_enc = pin_protocol + .encrypt(&shared_secret, &pin_hash("1234".as_bytes())) + .unwrap(); + let pin_req = CborRequest::try_from(&Ctap2ClientPinRequest::new_get_pin_token_with_perm( + Ctap2PinUvAuthProtocol::One, + public_key, + &pin_hash_enc, + getassertion.permissions(), + getassertion.permissions_rpid(), + )) + .unwrap(); + // We do here what the device would need to do, i.e. generate a new random + // pinUvAuthToken (here all 5's), then encrypt it using the shared_secret. + let token = [5; 32]; + let encrypted_token = pin_protocol.encrypt(&shared_secret, &token).unwrap(); + let pin_resp = CborResponse::new_success_from_slice( + to_vec(&Ctap2ClientPinResponse { + key_agreement: None, + pin_uv_auth_token: Some(ByteBuf::from(encrypted_token)), + pin_retries: None, + power_cycle_state: None, + uv_retries: None, + }) + .unwrap() + .as_slice(), + ); + channel.push_command_pair(pin_req, pin_resp); + + let mut recv = channel.get_ux_update_receiver(); + let recv_handle = tokio::task::spawn(async move { + if let UvUpdate::PinNotSet(update) = recv.recv().await.unwrap() { + assert_eq!(update.reason, PinNotSetReason::PinNotSet); + update.set_pin("1234").unwrap(); + } else { + panic!("Wrong UxUpdate received! Expected PinNotSet"); + } + if let UvUpdate::PinRequired(update) = recv.recv().await.unwrap() { + update.send_pin("1234").unwrap(); + } else { + panic!("Wrong UxUpdate received! Expected PinRequired"); + } + recv + }); + + // We should receive a PinNotSet-UvUpdate here before a clientPIN-request is issued + let resp = user_verification( + &mut channel, + UserVerificationRequirement::Required, + &mut getassertion, + TIMEOUT, + ) + .await; + + let expected_result = Ok(UsedPinUvAuthToken::NewlyCalculated( + Ctap2UserVerificationOperation::GetPinUvAuthTokenUsingPinWithPermissions, + )); + assert_eq!(resp, expected_result); + // Something ended up in the auth store + assert!(channel.get_auth_data().is_some()); + assert_eq!( + channel + .get_auth_data() + .as_ref() + .unwrap() + .pin_uv_auth_token + .as_ref() + .unwrap(), + &token + ); + assert_eq!( + channel.get_auth_data().unwrap().shared_secret, + shared_secret + ); + let recv = recv_handle.await.expect("Failed to join update thread"); + // No more updates should be sent + assert!(recv.is_empty()); + } + #[tokio::test] async fn early_exit_device_client_shared_secret_required_but_not_supported() { let testcases = vec![