Merge remote-tracking branch 'solokeys/main'

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

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "fido-authenticator"
-version = "0.1.0"
+version = "0.1.1"
 authors = ["Nicolas Stalder <[email protected]>"]
 edition = "2021"
 license = "Apache-2.0 OR MIT"

src/dispatch.rs

@@ -21,34 +21,44 @@ where
 
 #[inline(never)]
 /// Deserialize U2F, call authenticator, serialize response *Result*.
-fn handle_ctap1<T, UP>(
+fn handle_ctap1_from_hid<T, UP>(
     authenticator: &mut Authenticator<UP, T>,
     data: &[u8],
     response: &mut apdu_dispatch::response::Data,
 ) where
     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, UP>(
     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<T, UP>(
     authenticator: &mut Authenticator<UP, T>,
-    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)

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);

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);
             }