From d3a0d09f48cc39e24fbcc12e1af91b043ecefb62 Mon Sep 17 00:00:00 2001 From: "John M. Schanck" Date: Sun, 17 Sep 2023 13:25:17 -0700 Subject: [PATCH] Add StatusUpdate::SelectResultNotice --- examples/ctap2.rs | 3 + examples/ctap2_discoverable_creds.rs | 58 ++++++++++++--- examples/interactive_management.rs | 3 + examples/set_pin.rs | 3 + examples/test_exclude_list.rs | 3 + src/ctap2/commands/get_assertion.rs | 101 +++++++++++++-------------- src/ctap2/mod.rs | 31 ++++++-- src/ctap2/preflight.rs | 7 +- src/status_update.rs | 2 + src/transport/mod.rs | 2 +- 10 files changed, 140 insertions(+), 73 deletions(-) diff --git a/examples/ctap2.rs b/examples/ctap2.rs index f8e5350e..be742a5e 100644 --- a/examples/ctap2.rs +++ b/examples/ctap2.rs @@ -133,6 +133,9 @@ fn main() { Ok(StatusUpdate::PinUvError(e)) => { panic!("Unexpected error: {:?}", e) } + Ok(StatusUpdate::SelectResultNotice(_, _)) => { + panic!("Unexpected select device notice") + } Err(RecvError) => { println!("STATUS: end"); return; diff --git a/examples/ctap2_discoverable_creds.rs b/examples/ctap2_discoverable_creds.rs index 634fb30f..d19ccc6f 100644 --- a/examples/ctap2_discoverable_creds.rs +++ b/examples/ctap2_discoverable_creds.rs @@ -16,7 +16,8 @@ use authenticator::{ use getopts::Options; use sha2::{Digest, Sha256}; use std::sync::mpsc::{channel, RecvError}; -use std::{env, thread}; +use std::{env, io, thread}; +use std::io::Write; fn print_usage(program: &str, opts: Options) { println!("------------------------------------------------------------------------"); @@ -28,6 +29,37 @@ fn print_usage(program: &str, opts: Options) { print!("{}", opts.usage(&brief)); } +fn ask_user_choice(choices: &[PublicKeyCredentialUserEntity]) -> Option { + for (idx, op) in choices.iter().enumerate() { + println!("({idx}) \"{}\"", op.name.as_ref().unwrap()); + } + println!("({}) Cancel", choices.len()); + + let mut input = String::new(); + loop { + input.clear(); + print!("Your choice: "); + io::stdout() + .lock() + .flush() + .expect("Failed to flush stdout!"); + io::stdin() + .read_line(&mut input) + .expect("error: unable to read user input"); + if let Ok(idx) = input.trim().parse::() { + if idx < choices.len() { + // Add a newline in case of success for better separation of in/output + println!(); + return Some(idx); + } else if idx == choices.len() { + println!(); + return None; + } + println!("invalid input"); + } + } +} + fn register_user(manager: &mut AuthenticatorService, username: &str, timeout_ms: u64) { println!(); println!("*********************************************************************"); @@ -98,6 +130,9 @@ fn register_user(manager: &mut AuthenticatorService, username: &str, timeout_ms: Ok(StatusUpdate::PinUvError(e)) => { panic!("Unexpected error: {:?}", e) } + Ok(StatusUpdate::SelectResultNotice(_, _)) => { + panic!("Unexpected select result notice") + } Err(RecvError) => { println!("STATUS: end"); return; @@ -181,6 +216,10 @@ fn main() { "timeout in seconds", "SEC", ); + opts.optflag( + "s", + "skip_reg", + "Skip registration"); opts.optflag("h", "help", "print this help menu"); let matches = match opts.parse(&args[1..]) { @@ -208,8 +247,10 @@ fn main() { } }; - for username in &["A. User", "A. Nother", "Dr. Who"] { - register_user(&mut manager, username, timeout_ms) + if !matches.opt_present("skip_reg") { + for username in &["A. User", "A. Nother", "Dr. Who"] { + register_user(&mut manager, username, timeout_ms) + } } println!(); @@ -278,6 +319,11 @@ fn main() { Ok(StatusUpdate::PinUvError(e)) => { panic!("Unexpected error: {:?}", e) } + Ok(StatusUpdate::SelectResultNotice(index_sender, users)) => { + println!("Multiple signatures returned. Select one or cancel."); + let idx = ask_user_choice(&users); + index_sender.send(idx).expect("Failed to send choice"); + } Err(RecvError) => { println!("STATUS: end"); return; @@ -322,11 +368,7 @@ fn main() { println!("Found credentials:"); println!( "{:?}", - assertion_object - .assertions - .iter() - .map(|x| x.user.clone().unwrap().name.unwrap()) // Unwrapping here, as these shouldn't fail - .collect::>() + assertion_object.assertion.user.clone().unwrap().name.unwrap() // Unwrapping here, as these shouldn't fail ); println!("-----------------------------------------------------------------"); println!("Done."); diff --git a/examples/interactive_management.rs b/examples/interactive_management.rs index 7272eebe..6ef0e9dc 100644 --- a/examples/interactive_management.rs +++ b/examples/interactive_management.rs @@ -727,6 +727,9 @@ fn interactive_status_callback(status_rx: Receiver) { Ok(StatusUpdate::PinUvError(e)) => { panic!("Unexpected error: {:?}", e) } + Ok(StatusUpdate::SelectResultNotice(_, _)) => { + panic!("Unexpected select device notice") + } Err(RecvError) => { println!("STATUS: end"); return; diff --git a/examples/set_pin.rs b/examples/set_pin.rs index 2b204a32..5534ca08 100644 --- a/examples/set_pin.rs +++ b/examples/set_pin.rs @@ -114,6 +114,9 @@ fn main() { Ok(StatusUpdate::PinUvError(e)) => { panic!("Unexpected error: {:?}", e) } + Ok(StatusUpdate::SelectResultNotice(_, _)) => { + panic!("Unexpected select device notice") + } Err(RecvError) => { println!("STATUS: end"); return; diff --git a/examples/test_exclude_list.rs b/examples/test_exclude_list.rs index f7b0d131..e24c49d0 100644 --- a/examples/test_exclude_list.rs +++ b/examples/test_exclude_list.rs @@ -128,6 +128,9 @@ fn main() { Ok(StatusUpdate::PinUvError(e)) => { panic!("Unexpected error: {:?}", e) } + Ok(StatusUpdate::SelectResultNotice(_, _)) => { + panic!("Unexpected select device notice") + } Err(RecvError) => { println!("STATUS: end"); return; diff --git a/src/ctap2/commands/get_assertion.rs b/src/ctap2/commands/get_assertion.rs index 9074d0a3..d1988f7b 100644 --- a/src/ctap2/commands/get_assertion.rs +++ b/src/ctap2/commands/get_assertion.rs @@ -195,14 +195,10 @@ impl GetAssertion { // Handle extensions whose outputs are not encoded in the authenticator data. // 1. appId if let Some(app_id) = &self.extensions.app_id { - result.extensions.app_id = result - .assertions - .first() - .map(|assertion| { - assertion.auth_data.rp_id_hash - == RelyingPartyWrapper::from(app_id.as_str()).hash() - }) - .or(Some(false)); + result.extensions.app_id = Some( + result.assertion.auth_data.rp_id_hash + == RelyingPartyWrapper::from(app_id.as_str()).hash(), + ); } } } @@ -307,7 +303,7 @@ impl Serialize for GetAssertion { } impl RequestCtap1 for GetAssertion { - type Output = GetAssertionResult; + type Output = Vec; type AdditionalInfo = PublicKeyCredentialDescriptor; fn ctap1_format(&self) -> Result<(Vec, Self::AdditionalInfo), HIDError> { @@ -358,24 +354,27 @@ impl RequestCtap1 for GetAssertion { return Err(Retryable::Error(HIDError::ApduStatus(err))); } - let mut output = GetAssertionResult::from_ctap1(input, &self.rp.hash(), add_info) + let mut result = GetAssertionResult::from_ctap1(input, &self.rp.hash(), add_info) .map_err(|e| Retryable::Error(HIDError::Command(e)))?; - self.finalize_result(&mut output); - Ok(output) + self.finalize_result(&mut result); + // Although there's only one result, we return a vector for consistency with CTAP2. + Ok(vec![result]) } fn send_to_virtual_device( &self, dev: &mut Dev, ) -> Result { - let mut output = dev.get_assertion(self)?; - self.finalize_result(&mut output); - Ok(output) + let mut results = dev.get_assertion(self)?; + for result in results.iter_mut() { + self.finalize_result(result); + } + Ok(results) } } impl RequestCtap2 for GetAssertion { - type Output = GetAssertionResult; + type Output = Vec; fn command(&self) -> Command { Command::GetAssertion @@ -411,22 +410,27 @@ impl RequestCtap2 for GetAssertion { let assertion: GetAssertionResponse = from_slice(&input[1..]).map_err(CommandError::Deserializing)?; let number_of_credentials = assertion.number_of_credentials.unwrap_or(1); - let mut assertions = Vec::with_capacity(number_of_credentials); - assertions.push(assertion.into()); + + let mut results = Vec::with_capacity(number_of_credentials); + results.push(GetAssertionResult { + assertion: assertion.into(), + extensions: Default::default(), + }); let msg = GetNextAssertion; // We already have one, so skipping 0 for _ in 1..number_of_credentials { - let new_cred = dev.send_cbor(&msg)?; - assertions.push(new_cred.into()); + let assertion = dev.send_cbor(&msg)?; + results.push(GetAssertionResult { + assertion: assertion.into(), + extensions: Default::default(), + }); } - let mut output = GetAssertionResult { - assertions, - extensions: Default::default(), - }; - self.finalize_result(&mut output); - Ok(output) + for result in results.iter_mut() { + self.finalize_result(result); + } + Ok(results) } else { let data: Value = from_slice(&input[1..]).map_err(CommandError::Deserializing)?; Err(CommandError::StatusCode(status, Some(data)).into()) @@ -437,9 +441,11 @@ impl RequestCtap2 for GetAssertion { &self, dev: &mut Dev, ) -> Result { - let mut output = dev.get_assertion(self)?; - self.finalize_result(&mut output); - Ok(output) + let mut results = dev.get_assertion(self)?; + for result in results.iter_mut() { + self.finalize_result(result); + } + Ok(results) } } @@ -465,7 +471,7 @@ impl From for Assertion { #[derive(Debug, PartialEq, Eq)] pub struct GetAssertionResult { - pub assertions: Vec, + pub assertion: Assertion, pub extensions: AuthenticationExtensionsClientOutputs, } @@ -501,23 +507,10 @@ impl GetAssertionResult { }; Ok(GetAssertionResult { - assertions: vec![assertion], + assertion, extensions: Default::default(), }) } - - pub fn u2f_sign_data(&self) -> Vec { - if let Some(first) = self.assertions.first() { - let mut res = Vec::new(); - res.push(first.auth_data.flags.bits()); - res.extend(first.auth_data.counter.to_be_bytes()); - res.extend(&first.signature); - res - // first.signature.clone() - } else { - Vec::new() - } - } } pub struct GetAssertionResponse { @@ -791,10 +784,10 @@ pub mod test { auth_data: expected_auth_data, }; - let expected = GetAssertionResult { - assertions: vec![expected_assertion], + let expected = vec![GetAssertionResult { + assertion: expected_assertion, extensions: Default::default(), - }; + }]; let response = device.send_cbor(&assertion).unwrap(); assert_eq!(response, expected); } @@ -926,10 +919,10 @@ pub mod test { auth_data: expected_auth_data, }; - let expected = GetAssertionResult { - assertions: vec![expected_assertion], + let expected = vec![GetAssertionResult { + assertion: expected_assertion, extensions: Default::default(), - }; + }]; assert_eq!(response, expected); } @@ -1070,10 +1063,10 @@ pub mod test { auth_data: expected_auth_data, }; - let expected = GetAssertionResult { - assertions: vec![expected_assertion], + let expected = vec![GetAssertionResult { + assertion: expected_assertion, extensions: Default::default(), - }; + }]; assert_eq!(response, expected); } @@ -1338,7 +1331,7 @@ pub mod test { let resp = GetAssertionResult::from_ctap1(&sample, &rp_hash, &add_info) .expect("could not handle response"); assert_eq!( - resp.assertions[0].auth_data.flags, + resp.assertion.auth_data.flags, AuthenticatorDataFlags::USER_PRESENT | AuthenticatorDataFlags::RESERVED_1 ); } diff --git a/src/ctap2/mod.rs b/src/ctap2/mod.rs index 8c1e65b9..db5afd5f 100644 --- a/src/ctap2/mod.rs +++ b/src/ctap2/mod.rs @@ -680,15 +680,34 @@ pub fn sign( debug!("{get_assertion:?} using {pin_uv_auth_result:?}"); debug!("------------------------------------------------------------------"); send_status(&status, crate::StatusUpdate::PresenceRequired); - let resp = dev.send_msg_cancellable(&get_assertion, alive); - match resp { - Ok(result) => { - callback.call(Ok(result)); - return true; - } + let mut results = match dev.send_msg_cancellable(&get_assertion, alive) { + Ok(results) => results, Err(e) => { handle_errors!(e, status, callback, pin_uv_auth_result, skip_uv); } + }; + if results.len() == 1 { + callback.call(Ok(results.swap_remove(0))); + return true; + } + let (tx, rx) = channel(); + let user_entities = results + .iter() + .filter_map(|x| x.assertion.user.clone()) + .collect(); + send_status( + &status, + crate::StatusUpdate::SelectResultNotice(tx, user_entities), + ); + match rx.recv() { + Ok(Some(index)) if index < results.len() => { + callback.call(Ok(results.swap_remove(index))); + return true; + } + _ => { + callback.call(Err(AuthenticatorError::CancelledByUser)); + return true; + } } } false diff --git a/src/ctap2/preflight.rs b/src/ctap2/preflight.rs index 27ca4bf5..3ba1f989 100644 --- a/src/ctap2/preflight.rs +++ b/src/ctap2/preflight.rs @@ -161,13 +161,12 @@ pub(crate) fn do_credential_list_filtering_ctap2( ); silent_assert.set_pin_uv_auth_param(pin_uv_auth_token.clone())?; match dev.send_msg(&silent_assert) { - Ok(response) => { + Ok(mut response) => { // This chunk contains a key_handle that is already known to the device. // Filter out all credentials the device returned. Those are valid. let credential_ids = response - .assertions - .iter() - .filter_map(|a| a.credentials.clone()) + .iter_mut() + .filter_map(|result| result.assertion.credentials.take()) .collect(); // Replace credential_id_list with the valid credentials final_list = credential_ids; diff --git a/src/status_update.rs b/src/status_update.rs index f0306aea..c4a7fee7 100644 --- a/src/status_update.rs +++ b/src/status_update.rs @@ -105,6 +105,8 @@ pub enum StatusUpdate { SelectDeviceNotice, /// Sent when a token was selected for interactive management InteractiveManagement(InteractiveUpdate), + /// Sent when a token returns multiple results for a getAssertion request + SelectResultNotice(Sender>, Vec), } pub(crate) fn send_status(status: &Sender, msg: StatusUpdate) { diff --git a/src/transport/mod.rs b/src/transport/mod.rs index 55126057..52eea2ab 100644 --- a/src/transport/mod.rs +++ b/src/transport/mod.rs @@ -345,7 +345,7 @@ where pub trait VirtualFidoDevice: FidoDevice { fn check_key_handle(&self, req: &CheckKeyHandle) -> Result<(), HIDError>; fn client_pin(&self, req: &ClientPIN) -> Result; - fn get_assertion(&self, req: &GetAssertion) -> Result; + fn get_assertion(&self, req: &GetAssertion) -> Result, HIDError>; fn get_info(&self) -> Result; fn get_version(&self, req: &GetVersion) -> Result; fn make_credentials(&self, req: &MakeCredentials) -> Result;