Move protocol-specific user presence checking code from Env to CTAP library (#501)

* Common duration type for ctap library independent of TockOS

* Implement Env-specific ctap-hid channels for I/O
Common I/O Status, Error and Result types

* Move common user presence checking code to ctap library

* Move CtapHidChannel and UserPresence traits, with their accompanying
types to separate API mods. Remove Default implementations of methods
in these traits, to keep all implementation details inside of concrete
Env types.

Rename methods in UserPresence trait, for better readability.

Remove duplicate code for finding appropriate HID channel for given
transport.

Rework check_user_presence() function so that there's no more need for
quick_check() method in UserPresence trait. To short-circuit user
presence check, Env implementation may use wait_with_timeout() method.

* Fix button press wait with zero timeout for TockEnv

* Fix formatting

* Remove type for duration, use embedded_time::duration::Milliseconds
directly, for better readability.

Treat any unconfirmed result of user presence check as an error, which
maps more naturally to CTAP spec status codes.

Remove unneeded underscores in trait definition.

Store usb endpoint directly, in TockEnv channels, to avoid unneeded
conversions.

* No need for separate error type for send_keepalive_up_needed()

* Document UserPresence trait and types.

Remove unused parameters in UserPresence trait's methods.

Add conversion function from UserPresence errors to Ctap2 status codes.

Do not check button status when tock user presence wait is called with
zero timeout.

* Make test environment always report success sending data

* Rename CtapHidChannel to HidConnection, rename *_hid_channel ->
*_hid_connection, for clarity. Use "Channel" to refer to the logical
connection from authenticator to one client, and use "Connection" to
refer to physical connection of authenticator to platform, on which
clients run.

Remove channel parameter from user presence API, it's not needed.

* Remove duplicate comments.

Co-authored-by: kaczmarczyck <43844792+kaczmarczyck@users.noreply.github.com>

src/api/connection.rs

@@ -0,0 +1,34 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use crate::clock::ClockInt;
+use embedded_time::duration::Milliseconds;
+
+pub enum SendOrRecvStatus {
+    Timeout,
+    Sent,
+    Received,
+}
+
+pub struct SendOrRecvError;
+
+pub type SendOrRecvResult = Result<SendOrRecvStatus, SendOrRecvError>;
+
+pub trait HidConnection {
+    fn send_or_recv_with_timeout(
+        &mut self,
+        buf: &mut [u8; 64],
+        timeout: Milliseconds<ClockInt>,
+    ) -> SendOrRecvResult;
+}

src/api/mod.rs

@@ -3,6 +3,8 @@
 //! The [environment](crate::env::Env) is split into components. Each component has an API described
 //! by a trait. This module gathers the API of those components.
 
+pub mod connection;
 pub mod customization;
 pub mod firmware_protection;
 pub mod upgrade_storage;
+pub mod user_presence;

src/api/user_presence.rs

@@ -0,0 +1,44 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use crate::clock::ClockInt;
+use embedded_time::duration::Milliseconds;
+
+pub enum UserPresenceError {
+    /// User explicitly declined user presence check.
+    Declined,
+    /// User presence check was canceled by User Agent.
+    Canceled,
+    /// User presence check timed out.
+    Timeout,
+}
+
+pub type UserPresenceResult = Result<(), UserPresenceError>;
+
+pub trait UserPresence {
+    /// Initializes for a user presence check.
+    ///
+    /// Must eventually be followed by a call to [`Self::check_complete`].
+    fn check_init(&mut self);
+
+    /// Waits until user presence is confirmed, rejected, or the given timeout expires.
+    ///
+    /// Must be called between calls to [`Self::check_init`] and [`Self::check_complete`].
+    fn wait_with_timeout(&mut self, timeout: Milliseconds<ClockInt>) -> UserPresenceResult;
+
+    /// Finalizes a user presence check.
+    ///
+    /// Must be called after [`Self::check_init`].
+    fn check_complete(&mut self);
+}

src/clock.rs

@@ -24,7 +24,7 @@ impl<const CLOCK_FREQUENCY: u32> fmt::Debug for LibtockClock<CLOCK_FREQUENCY> {
     }
 }
 
-const KEEPALIVE_DELAY_MS: ClockInt = 100;
+pub const KEEPALIVE_DELAY_MS: ClockInt = 100;
 pub const KEEPALIVE_DELAY: Milliseconds<ClockInt> = Milliseconds(KEEPALIVE_DELAY_MS);
 
 #[cfg(target_pointer_width = "32")]

src/ctap/ctap1.rs

@@ -393,7 +393,7 @@ mod test {
     fn test_process_allowed() {
         let mut env = TestEnv::new();
         env.user_presence()
-            .set(|_| panic!("Unexpected user presence check in CTAP1"));
+            .set(|| panic!("Unexpected user presence check in CTAP1"));
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
         storage::toggle_always_uv(&mut env).unwrap();
 
@@ -410,7 +410,7 @@ mod test {
     fn test_process_register() {
         let mut env = TestEnv::new();
         env.user_presence()
-            .set(|_| panic!("Unexpected user presence check in CTAP1"));
+            .set(|| panic!("Unexpected user presence check in CTAP1"));
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let application = [0x0A; 32];
@@ -462,7 +462,7 @@ mod test {
     fn test_process_register_bad_message() {
         let mut env = TestEnv::new();
         env.user_presence()
-            .set(|_| panic!("Unexpected user presence check in CTAP1"));
+            .set(|| panic!("Unexpected user presence check in CTAP1"));
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let application = [0x0A; 32];
@@ -484,7 +484,7 @@ mod test {
 
         let mut env = TestEnv::new();
         env.user_presence()
-            .set(|_| panic!("Unexpected user presence check in CTAP1"));
+            .set(|| panic!("Unexpected user presence check in CTAP1"));
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         ctap_state.u2f_up_state.consume_up(CtapInstant::new(0));
@@ -500,7 +500,7 @@ mod test {
     fn test_process_authenticate_check_only() {
         let mut env = TestEnv::new();
         env.user_presence()
-            .set(|_| panic!("Unexpected user presence check in CTAP1"));
+            .set(|| panic!("Unexpected user presence check in CTAP1"));
         let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
@@ -518,7 +518,7 @@ mod test {
     fn test_process_authenticate_check_only_wrong_rp() {
         let mut env = TestEnv::new();
         env.user_presence()
-            .set(|_| panic!("Unexpected user presence check in CTAP1"));
+            .set(|| panic!("Unexpected user presence check in CTAP1"));
         let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
@@ -537,7 +537,7 @@ mod test {
     fn test_process_authenticate_check_only_wrong_length() {
         let mut env = TestEnv::new();
         env.user_presence()
-            .set(|_| panic!("Unexpected user presence check in CTAP1"));
+            .set(|| panic!("Unexpected user presence check in CTAP1"));
         let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
@@ -575,7 +575,7 @@ mod test {
     fn test_process_authenticate_check_only_wrong_cla() {
         let mut env = TestEnv::new();
         env.user_presence()
-            .set(|_| panic!("Unexpected user presence check in CTAP1"));
+            .set(|| panic!("Unexpected user presence check in CTAP1"));
         let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
@@ -595,7 +595,7 @@ mod test {
     fn test_process_authenticate_check_only_wrong_ins() {
         let mut env = TestEnv::new();
         env.user_presence()
-            .set(|_| panic!("Unexpected user presence check in CTAP1"));
+            .set(|| panic!("Unexpected user presence check in CTAP1"));
         let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
@@ -615,7 +615,7 @@ mod test {
     fn test_process_authenticate_check_only_wrong_flags() {
         let mut env = TestEnv::new();
         env.user_presence()
-            .set(|_| panic!("Unexpected user presence check in CTAP1"));
+            .set(|| panic!("Unexpected user presence check in CTAP1"));
         let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
@@ -643,7 +643,7 @@ mod test {
     fn test_process_authenticate_enforce() {
         let mut env = TestEnv::new();
         env.user_presence()
-            .set(|_| panic!("Unexpected user presence check in CTAP1"));
+            .set(|| panic!("Unexpected user presence check in CTAP1"));
         let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
@@ -671,7 +671,7 @@ mod test {
     fn test_process_authenticate_dont_enforce() {
         let mut env = TestEnv::new();
         env.user_presence()
-            .set(|_| panic!("Unexpected user presence check in CTAP1"));
+            .set(|| panic!("Unexpected user presence check in CTAP1"));
         let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
@@ -709,7 +709,7 @@ mod test {
 
         let mut env = TestEnv::new();
         env.user_presence()
-            .set(|_| panic!("Unexpected user presence check in CTAP1"));
+            .set(|| panic!("Unexpected user presence check in CTAP1"));
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         ctap_state.u2f_up_state.consume_up(CtapInstant::new(0));
@@ -728,7 +728,7 @@ mod test {
 
         let mut env = TestEnv::new();
         env.user_presence()
-            .set(|_| panic!("Unexpected user presence check in CTAP1"));
+            .set(|| panic!("Unexpected user presence check in CTAP1"));
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         ctap_state.u2f_up_state.consume_up(CtapInstant::new(0));

src/ctap/mod.rs

@@ -51,7 +51,7 @@ use self::data_formats::{
     PublicKeyCredentialSource, PublicKeyCredentialType, PublicKeyCredentialUserEntity,
     SignatureAlgorithm,
 };
-use self::hid::ChannelID;
+use self::hid::{ChannelID, CtapHid, CtapHidCommand, KeepaliveStatus, ProcessedPacket};
 use self::large_blobs::LargeBlobs;
 use self::response::{
     AuthenticatorGetAssertionResponse, AuthenticatorGetInfoResponse,
@@ -62,11 +62,13 @@ use self::status_code::Ctap2StatusCode;
 use self::timed_permission::TimedPermission;
 #[cfg(feature = "with_ctap1")]
 use self::timed_permission::U2fUserPresenceState;
+use crate::api::connection::{HidConnection, SendOrRecvStatus};
 use crate::api::customization::Customization;
 use crate::api::firmware_protection::FirmwareProtection;
 use crate::api::upgrade_storage::UpgradeStorage;
-use crate::clock::{ClockInt, CtapInstant};
+use crate::api::user_presence::{UserPresence, UserPresenceError};
-use crate::env::{Env, UserPresence};
+use crate::clock::{ClockInt, CtapInstant, KEEPALIVE_DELAY, KEEPALIVE_DELAY_MS};
+use crate::env::Env;
 use alloc::boxed::Box;
 use alloc::string::{String, ToString};
 use alloc::vec;
@@ -153,6 +155,16 @@ pub enum Transport {
     VendorHid,
 }
 
+impl Transport {
+    pub fn hid_connection<E: Env>(self, env: &mut E) -> &mut E::HidConnection {
+        match self {
+            Transport::MainHid => env.main_hid_connection(),
+            #[cfg(feature = "vendor_hid")]
+            Transport::VendorHid => env.vendor_hid_connection(),
+        }
+    }
+}
+
 /// Communication channels between authenticator and client.
 ///
 /// From OpenSK's perspective, a channel represents a client. When we receive data from a new
@@ -244,6 +256,106 @@ fn verify_signature(
     Ok(())
 }
 
+// Sends keepalive packet during user presence checking. If user agent replies with CANCEL response,
+// returns Err(UserPresenceError::Canceled).
+fn send_keepalive_up_needed(
+    env: &mut impl Env,
+    channel: Channel,
+    timeout: Milliseconds<ClockInt>,
+) -> Result<(), UserPresenceError> {
+    let (cid, transport) = match channel {
+        Channel::MainHid(cid) => (cid, Transport::MainHid),
+        #[cfg(feature = "vendor_hid")]
+        Channel::VendorHid(cid) => (cid, Transport::VendorHid),
+    };
+    let keepalive_msg = CtapHid::keepalive(cid, KeepaliveStatus::UpNeeded);
+    for mut pkt in keepalive_msg {
+        let ctap_hid_connection = transport.hid_connection(env);
+        match ctap_hid_connection.send_or_recv_with_timeout(&mut pkt, timeout) {
+            Ok(SendOrRecvStatus::Timeout) => {
+                debug_ctap!(env, "Sending a KEEPALIVE packet timed out");
+                // TODO: abort user presence test?
+            }
+            Err(_) => panic!("Error sending KEEPALIVE packet"),
+            Ok(SendOrRecvStatus::Sent) => {
+                debug_ctap!(env, "Sent KEEPALIVE packet");
+            }
+            Ok(SendOrRecvStatus::Received) => {
+                // We only parse one packet, because we only care about CANCEL.
+                let (received_cid, processed_packet) = CtapHid::process_single_packet(&pkt);
+                if received_cid != &cid {
+                    debug_ctap!(
+                        env,
+                        "Received a packet on channel ID {:?} while sending a KEEPALIVE packet",
+                        received_cid,
+                    );
+                    return Ok(());
+                }
+                match processed_packet {
+                    ProcessedPacket::InitPacket { cmd, .. } => {
+                        if cmd == CtapHidCommand::Cancel as u8 {
+                            // We ignore the payload, we can't answer with an error code anyway.
+                            debug_ctap!(env, "User presence check cancelled");
+                            return Err(UserPresenceError::Canceled);
+                        } else {
+                            debug_ctap!(
+                                env,
+                                "Discarded packet with command {} received while sending a KEEPALIVE packet",
+                                cmd,
+                            );
+                        }
+                    }
+                    ProcessedPacket::ContinuationPacket { .. } => {
+                        debug_ctap!(
+                            env,
+                            "Discarded continuation packet received while sending a KEEPALIVE packet",
+                        );
+                    }
+                }
+            }
+        }
+    }
+    Ok(())
+}
+
+/// Blocks for user presence.
+///
+/// Returns an error in case of timeout, user declining presence request, or keepalive error.
+fn check_user_presence(env: &mut impl Env, channel: Channel) -> Result<(), Ctap2StatusCode> {
+    env.user_presence().check_init();
+
+    // The timeout is N times the keepalive delay.
+    const TIMEOUT_ITERATIONS: usize = TOUCH_TIMEOUT_MS as usize / KEEPALIVE_DELAY_MS as usize;
+
+    // All fallible functions are called without '?' operator to always reach
+    // check_complete(...) cleanup function.
+
+    let mut result = Err(UserPresenceError::Timeout);
+    for i in 0..=TIMEOUT_ITERATIONS {
+        // First presence check is made without timeout. That way Env implementation may return
+        // user presence check result immediately to client, without sending any keepalive packets.
+        result = env.user_presence().wait_with_timeout(if i == 0 {
+            Milliseconds(0)
+        } else {
+            KEEPALIVE_DELAY
+        });
+        if !matches!(result, Err(UserPresenceError::Timeout)) {
+            break;
+        }
+        // TODO: this may take arbitrary time. Next wait's delay should be adjusted
+        // accordingly, so that all wait_with_timeout invocations are separated by
+        // equal time intervals. That way token indicators, such as LEDs, will blink
+        // with a consistent pattern.
+        result = send_keepalive_up_needed(env, channel, KEEPALIVE_DELAY);
+        if result.is_err() {
+            break;
+        }
+    }
+
+    env.user_presence().check_complete();
+    result.map_err(|e| e.into())
+}
+
 /// Holds data necessary to sign an assertion for a credential.
 #[derive(Clone)]
 pub struct AssertionInput {
@@ -590,7 +702,7 @@ impl CtapState {
         if let Some(auth_param) = &pin_uv_auth_param {
             // This case was added in FIDO 2.1.
             if auth_param.is_empty() {
-                env.user_presence().check(channel)?;
+                check_user_presence(env, channel)?;
                 if storage::pin_hash(env)?.is_none() {
                     return Err(Ctap2StatusCode::CTAP2_ERR_PIN_NOT_SET);
                 } else {
@@ -699,13 +811,13 @@ impl CtapState {
                 {
                     // Perform this check, so bad actors can't brute force exclude_list
                     // without user interaction.
-                    let _ = env.user_presence().check(channel);
+                    let _ = check_user_presence(env, channel);
                     return Err(Ctap2StatusCode::CTAP2_ERR_CREDENTIAL_EXCLUDED);
                 }
             }
         }
 
-        env.user_presence().check(channel)?;
+        check_user_presence(env, channel)?;
         self.client_pin.clear_token_flags();
 
         let default_cred_protect = env.customization().default_cred_protect();
@@ -1069,7 +1181,7 @@ impl CtapState {
 
         // This check comes before CTAP2_ERR_NO_CREDENTIALS in CTAP 2.0.
         if options.up {
-            env.user_presence().check(channel)?;
+            check_user_presence(env, channel)?;
             self.client_pin.clear_token_flags();
         }
 
@@ -1193,7 +1305,7 @@ impl CtapState {
             StatefulCommand::Reset => (),
             _ => return Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED),
         }
-        env.user_presence().check(channel)?;
+        check_user_presence(env, channel)?;
 
         storage::reset(env)?;
         self.client_pin.reset(env.rng());
@@ -1211,7 +1323,7 @@ impl CtapState {
         env: &mut impl Env,
         channel: Channel,
     ) -> Result<ResponseData, Ctap2StatusCode> {
-        env.user_presence().check(channel)?;
+        check_user_presence(env, channel)?;
         Ok(ResponseData::AuthenticatorSelection)
     }
 
@@ -1222,7 +1334,7 @@ impl CtapState {
         channel: Channel,
     ) -> Result<ResponseData, Ctap2StatusCode> {
         if params.attestation_material.is_some() || params.lockdown {
-            env.user_presence().check(channel)?;
+            check_user_presence(env, channel)?;
         }
 
         // Sanity checks
@@ -2124,8 +2236,7 @@ mod test {
     #[test]
     fn test_process_make_credential_cancelled() {
         let mut env = TestEnv::new();
-        env.user_presence()
+        env.user_presence().set(|| Err(UserPresenceError::Canceled));
-            .set(|_| Err(Ctap2StatusCode::CTAP2_ERR_KEEPALIVE_CANCEL));
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let make_credential_params = create_minimal_make_credential_parameters();
@@ -2916,8 +3027,7 @@ mod test {
     #[test]
     fn test_process_reset_cancelled() {
         let mut env = TestEnv::new();
-        env.user_presence()
+        env.user_presence().set(|| Err(UserPresenceError::Canceled));
-            .set(|_| Err(Ctap2StatusCode::CTAP2_ERR_KEEPALIVE_CANCEL));
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let reset_reponse = ctap_state.process_reset(&mut env, DUMMY_CHANNEL);

src/ctap/status_code.rs

@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use crate::api::user_presence::UserPresenceError;
+
 // CTAP specification (version 20190130) section 6.3
 // For now, only the CTAP2 codes are here, the CTAP1 are not included.
 #[allow(non_camel_case_types)]
@@ -81,3 +83,13 @@ pub enum Ctap2StatusCode {
     CTAP2_ERR_VENDOR_HARDWARE_FAILURE = 0xF3,
     _CTAP2_ERR_VENDOR_LAST = 0xFF,
 }
+
+impl From<UserPresenceError> for Ctap2StatusCode {
+    fn from(user_presence_error: UserPresenceError) -> Self {
+        match user_presence_error {
+            UserPresenceError::Timeout => Self::CTAP2_ERR_USER_ACTION_TIMEOUT,
+            UserPresenceError::Declined => Self::CTAP2_ERR_OPERATION_DENIED,
+            UserPresenceError::Canceled => Self::CTAP2_ERR_KEEPALIVE_CANCEL,
+        }
+    }
+}

src/env/mod.rs

@@ -1,8 +1,8 @@
+use crate::api::connection::HidConnection;
 use crate::api::customization::Customization;
 use crate::api::firmware_protection::FirmwareProtection;
 use crate::api::upgrade_storage::UpgradeStorage;
-use crate::ctap::status_code::Ctap2StatusCode;
+use crate::api::user_presence::UserPresence;
-use crate::ctap::Channel;
 use persistent_store::{Storage, Store};
 use rng256::Rng256;
 
@@ -10,13 +10,6 @@ use rng256::Rng256;
 pub mod test;
 pub mod tock;
 
-pub trait UserPresence {
-    /// Blocks for user presence.
-    ///
-    /// Returns an error in case of timeout or keepalive error.
-    fn check(&mut self, channel: Channel) -> Result<(), Ctap2StatusCode>;
-}
-
 /// Describes what CTAP needs to function.
 pub trait Env {
     type Rng: Rng256;
@@ -26,6 +19,7 @@ pub trait Env {
     type FirmwareProtection: FirmwareProtection;
     type Write: core::fmt::Write;
     type Customization: Customization;
+    type HidConnection: HidConnection;
 
     fn rng(&mut self) -> &mut Self::Rng;
     fn user_presence(&mut self) -> &mut Self::UserPresence;
@@ -48,4 +42,11 @@ pub trait Env {
     fn write(&mut self) -> Self::Write;
 
     fn customization(&self) -> &Self::Customization;
+
+    /// I/O connection for sending packets implementing CTAP HID protocol.
+    fn main_hid_connection(&mut self) -> &mut Self::HidConnection;
+
+    /// I/O connection for sending packets implementing vendor extensions to CTAP HID protocol.
+    #[cfg(feature = "vendor_hid")]
+    fn vendor_hid_connection(&mut self) -> &mut Self::HidConnection;
 }

src/env/test/mod.rs

@@ -1,10 +1,12 @@
 use self::upgrade_storage::BufferUpgradeStorage;
+use crate::api::connection::{HidConnection, SendOrRecvResult, SendOrRecvStatus};
 use crate::api::customization::DEFAULT_CUSTOMIZATION;
 use crate::api::firmware_protection::FirmwareProtection;
-use crate::ctap::status_code::Ctap2StatusCode;
+use crate::api::user_presence::{UserPresence, UserPresenceResult};
-use crate::ctap::Channel;
+use crate::clock::ClockInt;
-use crate::env::{Env, UserPresence};
+use crate::env::Env;
 use customization::TestCustomization;
+use embedded_time::duration::Milliseconds;
 use persistent_store::{BufferOptions, BufferStorage, Store};
 use rand::rngs::StdRng;
 use rand::{Rng, SeedableRng};
@@ -40,7 +42,7 @@ impl Rng256 for TestRng256 {
 }
 
 pub struct TestUserPresence {
-    check: Box<dyn Fn(Channel) -> Result<(), Ctap2StatusCode>>,
+    check: Box<dyn Fn() -> UserPresenceResult>,
 }
 
 pub struct TestWrite;
@@ -66,13 +68,24 @@ fn new_storage() -> BufferStorage {
     BufferStorage::new(store, options)
 }
 
+impl HidConnection for TestEnv {
+    fn send_or_recv_with_timeout(
+        &mut self,
+        _buf: &mut [u8; 64],
+        _timeout: Milliseconds<ClockInt>,
+    ) -> SendOrRecvResult {
+        // TODO: Implement I/O from canned requests/responses for integration testing.
+        Ok(SendOrRecvStatus::Sent)
+    }
+}
+
 impl TestEnv {
     pub fn new() -> Self {
         let rng = TestRng256 {
             rng: StdRng::seed_from_u64(0),
         };
         let user_presence = TestUserPresence {
-            check: Box::new(|_| Ok(())),
+            check: Box::new(|| Ok(())),
         };
         let storage = new_storage();
         let store = Store::new(storage).ok().unwrap();
@@ -101,15 +114,17 @@ impl TestEnv {
 }
 
 impl TestUserPresence {
-    pub fn set(&mut self, check: impl Fn(Channel) -> Result<(), Ctap2StatusCode> + 'static) {
+    pub fn set(&mut self, check: impl Fn() -> UserPresenceResult + 'static) {
         self.check = Box::new(check);
     }
 }
 
 impl UserPresence for TestUserPresence {
-    fn check(&mut self, channel: Channel) -> Result<(), Ctap2StatusCode> {
+    fn check_init(&mut self) {}
-        (self.check)(channel)
+    fn wait_with_timeout(&mut self, _timeout: Milliseconds<ClockInt>) -> UserPresenceResult {
+        (self.check)()
     }
+    fn check_complete(&mut self) {}
 }
 
 impl FirmwareProtection for TestEnv {
@@ -126,6 +141,7 @@ impl Env for TestEnv {
     type FirmwareProtection = Self;
     type Write = TestWrite;
     type Customization = TestCustomization;
+    type HidConnection = Self;
 
     fn rng(&mut self) -> &mut Self::Rng {
         &mut self.rng
@@ -154,4 +170,13 @@ impl Env for TestEnv {
     fn customization(&self) -> &Self::Customization {
         &self.customization
     }
+
+    fn main_hid_connection(&mut self) -> &mut Self::HidConnection {
+        self
+    }
+
+    #[cfg(feature = "vendor_hid")]
+    fn vendor_hid_connection(&mut self) -> &mut Self::HidConnection {
+        self
+    }
 }

src/env/tock/mod.rs

@@ -1,27 +1,61 @@
 pub use self::storage::{TockStorage, TockUpgradeStorage};
+use crate::api::connection::{HidConnection, SendOrRecvError, SendOrRecvResult, SendOrRecvStatus};
 use crate::api::customization::{CustomizationImpl, DEFAULT_CUSTOMIZATION};
 use crate::api::firmware_protection::FirmwareProtection;
-use crate::ctap::hid::{CtapHid, CtapHidCommand, KeepaliveStatus, ProcessedPacket};
+use crate::api::user_presence::{UserPresence, UserPresenceError, UserPresenceResult};
-use crate::ctap::status_code::Ctap2StatusCode;
+use crate::clock::{ClockInt, KEEPALIVE_DELAY_MS};
-use crate::ctap::Channel;
+use crate::env::Env;
-use crate::env::{Env, UserPresence};
 use core::cell::Cell;
 use core::sync::atomic::{AtomicBool, Ordering};
+use embedded_time::duration::Milliseconds;
+use embedded_time::fixed_point::FixedPoint;
 use libtock_core::result::{CommandError, EALREADY};
 use libtock_drivers::buttons::{self, ButtonState};
 use libtock_drivers::console::Console;
 use libtock_drivers::result::{FlexUnwrap, TockError};
 use libtock_drivers::timer::Duration;
-use libtock_drivers::{crp, led, timer, usb_ctap_hid};
+use libtock_drivers::usb_ctap_hid::{self, UsbEndpoint};
+use libtock_drivers::{crp, led, timer};
 use persistent_store::{StorageResult, Store};
 use rng256::TockRng256;
 
 mod storage;
 
+pub struct TockHidConnection {
+    endpoint: UsbEndpoint,
+}
+
+impl HidConnection for TockHidConnection {
+    fn send_or_recv_with_timeout(
+        &mut self,
+        buf: &mut [u8; 64],
+        timeout: Milliseconds<ClockInt>,
+    ) -> SendOrRecvResult {
+        match usb_ctap_hid::send_or_recv_with_timeout(
+            buf,
+            timer::Duration::from_ms(timeout.integer() as isize),
+            self.endpoint,
+        ) {
+            Ok(usb_ctap_hid::SendOrRecvStatus::Timeout) => Ok(SendOrRecvStatus::Timeout),
+            Ok(usb_ctap_hid::SendOrRecvStatus::Sent) => Ok(SendOrRecvStatus::Sent),
+            Ok(usb_ctap_hid::SendOrRecvStatus::Received(recv_endpoint))
+                if self.endpoint == recv_endpoint =>
+            {
+                Ok(SendOrRecvStatus::Received)
+            }
+            _ => Err(SendOrRecvError),
+        }
+    }
+}
+
 pub struct TockEnv {
     rng: TockRng256,
     store: Store<TockStorage>,
     upgrade_storage: Option<TockUpgradeStorage>,
+    main_connection: TockHidConnection,
+    #[cfg(feature = "vendor_hid")]
+    vendor_connection: TockHidConnection,
+    blink_pattern: usize,
 }
 
 impl TockEnv {
@@ -39,6 +73,14 @@ impl TockEnv {
             rng: TockRng256 {},
             store,
             upgrade_storage,
+            main_connection: TockHidConnection {
+                endpoint: UsbEndpoint::MainHid,
+            },
+            #[cfg(feature = "vendor_hid")]
+            vendor_connection: TockHidConnection {
+                endpoint: UsbEndpoint::VendorHid,
+            },
+            blink_pattern: 0,
         }
     }
 }
@@ -56,8 +98,71 @@ pub fn take_storage() -> StorageResult<TockStorage> {
 }
 
 impl UserPresence for TockEnv {
-    fn check(&mut self, channel: Channel) -> Result<(), Ctap2StatusCode> {
+    fn check_init(&mut self) {
-        check_user_presence(self, channel)
+        self.blink_pattern = 0;
+    }
+    fn wait_with_timeout(&mut self, timeout: Milliseconds<ClockInt>) -> UserPresenceResult {
+        if timeout.integer() == 0 {
+            return Err(UserPresenceError::Timeout);
+        }
+        blink_leds(self.blink_pattern);
+        self.blink_pattern += 1;
+
+        let button_touched = Cell::new(false);
+        let mut buttons_callback = buttons::with_callback(|_button_num, state| {
+            match state {
+                ButtonState::Pressed => button_touched.set(true),
+                ButtonState::Released => (),
+            };
+        });
+        let mut buttons = buttons_callback.init().flex_unwrap();
+        for mut button in &mut buttons {
+            button.enable().flex_unwrap();
+        }
+
+        // Setup a keep-alive callback.
+        let keepalive_expired = Cell::new(false);
+        let mut keepalive_callback = timer::with_callback(|_, _| {
+            keepalive_expired.set(true);
+        });
+        let mut keepalive = keepalive_callback.init().flex_unwrap();
+        let keepalive_alarm = keepalive
+            .set_alarm(timer::Duration::from_ms(timeout.integer() as isize))
+            .flex_unwrap();
+
+        // Wait for a button touch or an alarm.
+        libtock_drivers::util::yieldk_for(|| button_touched.get() || keepalive_expired.get());
+
+        // Cleanup alarm callback.
+        match keepalive.stop_alarm(keepalive_alarm) {
+            Ok(()) => (),
+            Err(TockError::Command(CommandError {
+                return_code: EALREADY,
+                ..
+            })) => assert!(keepalive_expired.get()),
+            Err(_e) => {
+                #[cfg(feature = "debug_ctap")]
+                panic!("Unexpected error when stopping alarm: {:?}", _e);
+                #[cfg(not(feature = "debug_ctap"))]
+                panic!("Unexpected error when stopping alarm: <error is only visible with the debug_ctap feature>");
+            }
+        }
+
+        for mut button in &mut buttons {
+            button.disable().flex_unwrap();
+        }
+
+        if button_touched.get() {
+            Ok(())
+        } else if keepalive_expired.get() {
+            Err(UserPresenceError::Timeout)
+        } else {
+            panic!("Unexpected exit condition");
+        }
+    }
+
+    fn check_complete(&mut self) {
+        switch_off_leds();
     }
 }
 
@@ -82,6 +187,7 @@ impl Env for TockEnv {
     type FirmwareProtection = Self;
     type Write = Console;
     type Customization = CustomizationImpl;
+    type HidConnection = TockHidConnection;
 
     fn rng(&mut self) -> &mut Self::Rng {
         &mut self.rng
@@ -110,67 +216,15 @@ impl Env for TockEnv {
     fn customization(&self) -> &Self::Customization {
         &DEFAULT_CUSTOMIZATION
     }
+
+    fn main_hid_connection(&mut self) -> &mut Self::HidConnection {
+        &mut self.main_connection
     }
 
-// Returns whether the keepalive was sent, or false if cancelled.
-fn send_keepalive_up_needed(
-    env: &mut TockEnv,
-    channel: Channel,
-    timeout: Duration<isize>,
-) -> Result<(), Ctap2StatusCode> {
-    let (endpoint, cid) = match channel {
-        Channel::MainHid(cid) => (usb_ctap_hid::UsbEndpoint::MainHid, cid),
     #[cfg(feature = "vendor_hid")]
-        Channel::VendorHid(cid) => (usb_ctap_hid::UsbEndpoint::VendorHid, cid),
+    fn vendor_hid_connection(&mut self) -> &mut Self::HidConnection {
-    };
+        &mut self.vendor_connection
-    let keepalive_msg = CtapHid::keepalive(cid, KeepaliveStatus::UpNeeded);
-    for mut pkt in keepalive_msg {
-        let status =
-            usb_ctap_hid::send_or_recv_with_timeout(&mut pkt, timeout, endpoint).flex_unwrap();
-        match status {
-            usb_ctap_hid::SendOrRecvStatus::Timeout => {
-                debug_ctap!(env, "Sending a KEEPALIVE packet timed out");
-                // TODO: abort user presence test?
     }
-            usb_ctap_hid::SendOrRecvStatus::Sent => {
-                debug_ctap!(env, "Sent KEEPALIVE packet");
-            }
-            usb_ctap_hid::SendOrRecvStatus::Received(received_endpoint) => {
-                // We only parse one packet, because we only care about CANCEL.
-                let (received_cid, processed_packet) = CtapHid::process_single_packet(&pkt);
-                if received_endpoint != endpoint || received_cid != &cid {
-                    debug_ctap!(
-                        env,
-                        "Received a packet on channel ID {:?} while sending a KEEPALIVE packet",
-                        received_cid,
-                    );
-                    return Ok(());
-                }
-                match processed_packet {
-                    ProcessedPacket::InitPacket { cmd, .. } => {
-                        if cmd == CtapHidCommand::Cancel as u8 {
-                            // We ignore the payload, we can't answer with an error code anyway.
-                            debug_ctap!(env, "User presence check cancelled");
-                            return Err(Ctap2StatusCode::CTAP2_ERR_KEEPALIVE_CANCEL);
-                        } else {
-                            debug_ctap!(
-                                env,
-                                "Discarded packet with command {} received while sending a KEEPALIVE packet",
-                                cmd,
-                            );
-                        }
-                    }
-                    ProcessedPacket::ContinuationPacket { .. } => {
-                        debug_ctap!(
-                            env,
-                            "Discarded continuation packet received while sending a KEEPALIVE packet",
-                        );
-                    }
-                }
-            }
-        }
-    }
-    Ok(())
 }
 
 pub fn blink_leds(pattern_seed: usize) {
@@ -222,86 +276,4 @@ pub fn switch_off_leds() {
     }
 }
 
-const KEEPALIVE_DELAY_MS: isize = 100;
+pub const KEEPALIVE_DELAY_TOCK: Duration<isize> = Duration::from_ms(KEEPALIVE_DELAY_MS as isize);
-pub const KEEPALIVE_DELAY_TOCK: Duration<isize> = Duration::from_ms(KEEPALIVE_DELAY_MS);
-
-fn check_user_presence(env: &mut TockEnv, channel: Channel) -> Result<(), Ctap2StatusCode> {
-    // The timeout is N times the keepalive delay.
-    const TIMEOUT_ITERATIONS: usize =
-        crate::ctap::TOUCH_TIMEOUT_MS as usize / KEEPALIVE_DELAY_MS as usize;
-
-    // First, send a keep-alive packet to notify that the keep-alive status has changed.
-    send_keepalive_up_needed(env, channel, KEEPALIVE_DELAY_TOCK)?;
-
-    // Listen to the button presses.
-    let button_touched = Cell::new(false);
-    let mut buttons_callback = buttons::with_callback(|_button_num, state| {
-        match state {
-            ButtonState::Pressed => button_touched.set(true),
-            ButtonState::Released => (),
-        };
-    });
-    let mut buttons = buttons_callback.init().flex_unwrap();
-    // At the moment, all buttons are accepted. You can customize your setup here.
-    for mut button in &mut buttons {
-        button.enable().flex_unwrap();
-    }
-
-    let mut keepalive_response = Ok(());
-    for i in 0..TIMEOUT_ITERATIONS {
-        blink_leds(i);
-
-        // Setup a keep-alive callback.
-        let keepalive_expired = Cell::new(false);
-        let mut keepalive_callback = timer::with_callback(|_, _| {
-            keepalive_expired.set(true);
-        });
-        let mut keepalive = keepalive_callback.init().flex_unwrap();
-        let keepalive_alarm = keepalive.set_alarm(KEEPALIVE_DELAY_TOCK).flex_unwrap();
-
-        // Wait for a button touch or an alarm.
-        libtock_drivers::util::yieldk_for(|| button_touched.get() || keepalive_expired.get());
-
-        // Cleanup alarm callback.
-        match keepalive.stop_alarm(keepalive_alarm) {
-            Ok(()) => (),
-            Err(TockError::Command(CommandError {
-                return_code: EALREADY,
-                ..
-            })) => assert!(keepalive_expired.get()),
-            Err(_e) => {
-                #[cfg(feature = "debug_ctap")]
-                panic!("Unexpected error when stopping alarm: {:?}", _e);
-                #[cfg(not(feature = "debug_ctap"))]
-                panic!("Unexpected error when stopping alarm: <error is only visible with the debug_ctap feature>");
-            }
-        }
-
-        // TODO: this may take arbitrary time. The keepalive_delay should be adjusted accordingly,
-        // so that LEDs blink with a consistent pattern.
-        if keepalive_expired.get() {
-            // Do not return immediately, because we must clean up still.
-            keepalive_response = send_keepalive_up_needed(env, channel, KEEPALIVE_DELAY_TOCK);
-        }
-
-        if button_touched.get() || keepalive_response.is_err() {
-            break;
-        }
-    }
-
-    switch_off_leds();
-
-    // Cleanup button callbacks.
-    for mut button in &mut buttons {
-        button.disable().flex_unwrap();
-    }
-
-    // Returns whether the user was present.
-    if keepalive_response.is_err() {
-        keepalive_response
-    } else if button_touched.get() {
-        Ok(())
-    } else {
-        Err(Ctap2StatusCode::CTAP2_ERR_USER_ACTION_TIMEOUT)
-    }
-}

src/lib.rs

@@ -90,7 +90,6 @@ impl<E: Env> Ctap<E> {
         &mut self.hid
     }
 
-    #[cfg(feature = "std")]
     pub fn env(&mut self) -> &mut E {
         &mut self.env
     }

src/main.rs

@@ -28,12 +28,13 @@ use core::convert::TryFrom;
 use core::convert::TryInto;
 #[cfg(feature = "debug_ctap")]
 use core::fmt::Write;
+use ctap2::api::connection::{HidConnection, SendOrRecvStatus};
 #[cfg(feature = "debug_ctap")]
 use ctap2::clock::CtapClock;
-use ctap2::clock::{new_clock, Clock, ClockInt, KEEPALIVE_DELAY};
+use ctap2::clock::{new_clock, Clock, ClockInt, KEEPALIVE_DELAY, KEEPALIVE_DELAY_MS};
 #[cfg(feature = "with_ctap1")]
 use ctap2::env::tock::blink_leds;
-use ctap2::env::tock::{switch_off_leds, wink_leds, TockEnv, KEEPALIVE_DELAY_TOCK};
+use ctap2::env::tock::{switch_off_leds, wink_leds, TockEnv};
 use ctap2::Transport;
 #[cfg(feature = "debug_ctap")]
 use embedded_time::duration::Microseconds;
@@ -48,7 +49,8 @@ use libtock_drivers::usb_ctap_hid;
 
 libtock_core::stack_size! {0x4000}
 
-const SEND_TIMEOUT: Duration<isize> = Duration::from_ms(1000);
+const SEND_TIMEOUT: Milliseconds<ClockInt> = Milliseconds(1000);
+const KEEPALIVE_DELAY_TOCK: Duration<isize> = Duration::from_ms(KEEPALIVE_DELAY_MS as isize);
 
 fn main() {
     let clock = new_clock();
@@ -132,26 +134,25 @@ fn main() {
             let reply = ctap.process_hid_packet(&pkt_request, transport, now);
             // This block handles sending packets.
             for mut pkt_reply in reply {
-                let status =
+                let hid_connection = transport.hid_connection(ctap.env());
-                    usb_ctap_hid::send_or_recv_with_timeout(&mut pkt_reply, SEND_TIMEOUT, endpoint)
+                match hid_connection.send_or_recv_with_timeout(&mut pkt_reply, SEND_TIMEOUT) {
-                        .flex_unwrap();
+                    Ok(SendOrRecvStatus::Timeout) => {
-                match status {
-                    usb_ctap_hid::SendOrRecvStatus::Timeout => {
                         #[cfg(feature = "debug_ctap")]
                         print_packet_notice("Sending packet timed out", &clock);
                         // TODO: reset the ctap_hid state.
                         // Since sending the packet timed out, we cancel this reply.
                         break;
                     }
-                    usb_ctap_hid::SendOrRecvStatus::Sent => {
+                    Ok(SendOrRecvStatus::Sent) => {
                         #[cfg(feature = "debug_ctap")]
                         print_packet_notice("Sent packet", &clock);
                     }
-                    usb_ctap_hid::SendOrRecvStatus::Received(_) => {
+                    Ok(SendOrRecvStatus::Received) => {
                         #[cfg(feature = "debug_ctap")]
                         print_packet_notice("Received an UNEXPECTED packet", &clock);
                         // TODO: handle this unexpected packet.
                     }
+                    Err(_) => panic!("Error sending packet"),
                 }
             }
         }