From 60f54e5dbd00880607559a1b8fa28c16a077dd46 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Sun, 21 Aug 2022 21:58:21 -0700 Subject: [PATCH] dont treat iso14443 as apdu over apdu --- src/dispatch.rs | 67 +++++++++++++++++++++++------------------ src/dispatch/apdu.rs | 4 +-- src/dispatch/ctaphid.rs | 2 +- 3 files changed, 41 insertions(+), 32 deletions(-) diff --git a/src/dispatch.rs b/src/dispatch.rs index e1da024..fb65133 100644 --- a/src/dispatch.rs +++ b/src/dispatch.rs @@ -21,7 +21,7 @@ where #[inline(never)] /// Deserialize U2F, call authenticator, serialize response *Result*. -fn handle_ctap1( +fn handle_ctap1_from_hid( authenticator: &mut Authenticator, data: &[u8], response: &mut apdu_dispatch::response::Data, @@ -29,26 +29,36 @@ fn handle_ctap1( T: TrussedRequirements, UP: UserPresence, { - debug_now!( + debug!( "handle CTAP1: remaining stack: {} bytes", msp() - 0x2000_0000 ); - // debug_now!("1A SP: {:X}", msp()); - match try_handle_ctap1(authenticator, data, response) { - Ok(()) => { - debug!("U2F response {} bytes", response.len()); - // Need to add x9000 success code (normally the apdu-dispatch does this, but - // since u2f uses apdus over ctaphid, we must do it here.) - response.extend_from_slice(&[0x90, 0x00]).ok(); - } - Err(status) => { - let code: [u8; 2] = status.into(); - debug_now!("CTAP1 error: {:?} ({})", status, hex_str!(&code)); + { + let command = apdu_dispatch::Command::try_from(data); + if let Err(status) = command { + let code: [u8; 2] = (Status::IncorrectDataParameter).into(); + debug!("CTAP1 parse error: {:?} ({})", status, hex_str!(&code)); response.extend_from_slice(&code).ok(); + return; + } + + // debug!("1A SP: {:X}", msp()); + match try_handle_ctap1(authenticator, &command.unwrap(), response) { + Ok(()) => { + debug!("U2F response {} bytes", response.len()); + // Need to add x9000 success code (normally the apdu-dispatch does this, but + // since u2f uses apdus over ctaphid, we must do it here.) + response.extend_from_slice(&[0x90, 0x00]).ok(); + } + Err(status) => { + let code: [u8; 2] = status.into(); + debug!("CTAP1 error: {:?} ({})", status, hex_str!(&code)); + response.extend_from_slice(&code).ok(); + } } } - // debug_now!("1B SP: {:X}", msp()); - debug_now!("end handle CTAP1"); + // debug!("1B SP: {:X}", msp()); + debug!("end handle CTAP1"); } #[inline(never)] @@ -61,23 +71,25 @@ fn handle_ctap2( T: TrussedRequirements, UP: UserPresence, { - debug_now!( + debug!( "handle CTAP2: remaining stack: {} bytes", msp() - 0x2000_0000 ); - // debug_now!("2A SP: {:X}", msp()); + + debug!("1a SP: {:X}", msp()); + // debug!("2A SP: {:X}", msp()); if let Err(error) = try_handle_ctap2(authenticator, data, response) { - debug_now!("CTAP2 error: {:02X}", error); + debug!("CTAP2 error: {:02X}", error); response.push(error).ok(); } - // debug_now!("2B SP: {:X}", msp()); - debug_now!("end handle CTAP2"); + // debug!("2B SP: {:X}", msp()); + debug!("end handle CTAP2"); } #[inline(never)] fn try_handle_ctap1( authenticator: &mut Authenticator, - data: &[u8], + command: &apdu_dispatch::Command, response: &mut apdu_dispatch::response::Data, ) -> Result<(), Status> where @@ -101,14 +113,11 @@ where // Goal of these nested scopes is to keep stack small. let ctap_response = { let ctap_request = { - let command = apdu_dispatch::Command::try_from(data) - .map_err(|_| Status::IncorrectDataParameter)?; - // debug_now!("1a SP: {:X}", msp()); - ctap1::Request::try_from(&command)? + ctap1::Request::try_from(command)? }; ctap1::Authenticator::call_ctap1(authenticator, &ctap_request)? }; - // debug_now!("1b SP: {:X}", msp()); + // debug!("1b SP: {:X}", msp()); ctap_response.serialize(response).ok(); Ok(()) @@ -130,7 +139,7 @@ where .persistent .load_if_not_initialised(&mut authenticator.trussed); - debug_now!( + debug!( "try_handle CTAP2: remaining stack: {} bytes", msp() - 0x2000_0000 ); @@ -161,14 +170,14 @@ where .persistent .load_if_not_initialised(&mut authenticator.trussed); - debug_now!( + debug!( "try_get CTAP2: remaining stack: {} bytes", msp() - 0x2000_0000 ); // Goal of these nested scopes is to keep stack small. let ctap_request = ctap2::Request::deserialize(data).map_err(|error| error as u8)?; - debug_now!("2a SP: {:X}", msp()); + debug!("2a SP: {:X}", msp()); use ctap2::Authenticator; authenticator .call_ctap2(&ctap_request) diff --git a/src/dispatch/apdu.rs b/src/dispatch/apdu.rs index 7d4e834..3cfc42d 100644 --- a/src/dispatch/apdu.rs +++ b/src/dispatch/apdu.rs @@ -59,13 +59,13 @@ where Ok(match instruction { // U2F instruction codes // NB(nickray): I don't think 0x00 is a valid case. - 0x00 | 0x01 | 0x02 => super::handle_ctap1(self, apdu.data(), response), //self.call_authenticator_u2f(apdu, response), + 0x00 | 0x01 | 0x02 => super::try_handle_ctap1(self, apdu, response)?, //self.call_authenticator_u2f(apdu, response), _ => { match ctaphid::Command::try_from(instruction) { // 0x10 Ok(ctaphid::Command::Cbor) => super::handle_ctap2(self, apdu.data(), response), - Ok(ctaphid::Command::Msg) => super::handle_ctap1(self, apdu.data(), response), + Ok(ctaphid::Command::Msg) => super::try_handle_ctap1(self, apdu, response)?, Ok(ctaphid::Command::Deselect) => self.deselect(), _ => { info!("Unsupported ins for fido app {:02x}", instruction); diff --git a/src/dispatch/ctaphid.rs b/src/dispatch/ctaphid.rs index fac824c..e71b7c3 100644 --- a/src/dispatch/ctaphid.rs +++ b/src/dispatch/ctaphid.rs @@ -34,7 +34,7 @@ where // blocking::dump_hex(request, request.len()); Ok(match command { ctaphid::Command::Cbor => super::handle_ctap2(self, request, response), - ctaphid::Command::Msg => super::handle_ctap1(self, request, response), + ctaphid::Command::Msg => super::handle_ctap1_from_hid(self, request, response), _ => { debug_now!("ctaphid trying to dispatch {:?}", command); }