diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a81ef9..e826f7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.1.1] - 2022-08-22 +- Fix bug that treated U2F payloads as APDU over APDU in NFC transport @conorpp +- Add config option to skip UP when device was just booted, + as insertion is a kind of UP check @robin-nitrokey + ## [Unreleased] - use 2021 edition diff --git a/Cargo.toml b/Cargo.toml index 83ece9b..a9124f0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "fido-authenticator" -version = "0.1.0" +version = "0.1.1" authors = ["Nicolas Stalder "] edition = "2021" license = "Apache-2.0 OR MIT" diff --git a/src/dispatch.rs b/src/dispatch.rs index e1da024..3393f6c 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 @@ -100,15 +112,10 @@ 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)? - }; + let ctap_request = 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 +137,7 @@ where .persistent .load_if_not_initialised(&mut authenticator.trussed); - debug_now!( + debug!( "try_handle CTAP2: remaining stack: {} bytes", msp() - 0x2000_0000 ); @@ -161,14 +168,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 11b4be6..cbc2ed4 100644 --- a/src/dispatch/apdu.rs +++ b/src/dispatch/apdu.rs @@ -59,13 +59,13 @@ where 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 d27558e..60f0c8e 100644 --- a/src/dispatch/ctaphid.rs +++ b/src/dispatch/ctaphid.rs @@ -34,7 +34,7 @@ where // blocking::dump_hex(request, request.len()); 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); }