diff --git a/Cargo.lock b/Cargo.lock index 08ff0630..0986339f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -412,6 +412,7 @@ name = "libtock_drivers" version = "0.1.0" dependencies = [ "libtock_alarm", + "libtock_buttons", "libtock_console", "libtock_platform", ] diff --git a/libraries/opensk/src/api/connection.rs b/libraries/opensk/src/api/connection.rs index b3a577ce..5d7e0f93 100644 --- a/libraries/opensk/src/api/connection.rs +++ b/libraries/opensk/src/api/connection.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use crate::ctap::status_code::{Ctap2StatusCode, CtapResult}; use core::convert::TryFrom; #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -22,31 +23,26 @@ pub enum UsbEndpoint { } impl TryFrom for UsbEndpoint { - type Error = SendOrRecvError; + type Error = Ctap2StatusCode; - fn try_from(endpoint_num: usize) -> Result { + fn try_from(endpoint_num: usize) -> CtapResult { match endpoint_num { 1 => Ok(UsbEndpoint::MainHid), #[cfg(feature = "vendor_hid")] 2 => Ok(UsbEndpoint::VendorHid), - _ => Err(SendOrRecvError), + _ => Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_HARDWARE_FAILURE), } } } -pub enum SendOrRecvStatus { +pub enum RecvStatus { Timeout, - Sent, Received(UsbEndpoint), } -#[derive(Debug, PartialEq, Eq)] -pub struct SendOrRecvError; - -pub type SendOrRecvResult = Result; - pub trait HidConnection { - fn send_and_maybe_recv(&mut self, buf: &mut [u8; 64], timeout_ms: usize) -> SendOrRecvResult; + fn send(&mut self, buf: &[u8; 64], endpoint: UsbEndpoint) -> CtapResult<()>; + fn recv(&mut self, buf: &mut [u8; 64], timeout_ms: usize) -> CtapResult; } #[cfg(test)] @@ -58,6 +54,9 @@ mod test { assert_eq!(UsbEndpoint::try_from(1), Ok(UsbEndpoint::MainHid)); #[cfg(feature = "vendor_hid")] assert_eq!(UsbEndpoint::try_from(2), Ok(UsbEndpoint::VendorHid)); - assert_eq!(UsbEndpoint::try_from(3), Err(SendOrRecvError)); + assert_eq!( + UsbEndpoint::try_from(3), + Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_HARDWARE_FAILURE) + ); } } diff --git a/libraries/opensk/src/api/user_presence.rs b/libraries/opensk/src/api/user_presence.rs index 6b0dbbe6..dc9e9557 100644 --- a/libraries/opensk/src/api/user_presence.rs +++ b/libraries/opensk/src/api/user_presence.rs @@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +use crate::api::connection::RecvStatus; +use crate::ctap::status_code::CtapResult; + #[derive(Debug)] pub enum UserPresenceError { /// User explicitly declined user presence check. @@ -20,12 +23,11 @@ pub enum UserPresenceError { Canceled, /// User presence check timed out. Timeout, - /// Unexpected (e.g., hardware) failures - Fail, } - pub type UserPresenceResult = Result<(), UserPresenceError>; +pub type UserPresenceWaitResult = CtapResult<(UserPresenceResult, RecvStatus)>; + pub trait UserPresence { /// Initializes for a user presence check. /// @@ -35,7 +37,13 @@ pub trait UserPresence { /// 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_ms: usize) -> UserPresenceResult; + /// The function may write to the packet buffer, if it receives one during the wait. + /// If it does, the returned option contains the endpoint it received the data from. + fn wait_with_timeout( + &mut self, + packet: &mut [u8; 64], + timeout_ms: usize, + ) -> UserPresenceWaitResult; /// Finalizes a user presence check. /// diff --git a/libraries/opensk/src/ctap/hid/mod.rs b/libraries/opensk/src/ctap/hid/mod.rs index 955b949f..4ad1dbc4 100644 --- a/libraries/opensk/src/ctap/hid/mod.rs +++ b/libraries/opensk/src/ctap/hid/mod.rs @@ -440,6 +440,11 @@ impl CtapHid { }) } + /// Generates the HID response packets for a busy error. + pub fn busy_error(cid: ChannelID) -> HidPacketIterator { + Self::split_message(Self::error_message(cid, CtapHidError::ChannelBusy)) + } + #[cfg(test)] pub fn new_initialized() -> (Self, ChannelID) { ( @@ -633,6 +638,25 @@ mod test { } } + #[test] + fn test_busy_error() { + let cid = [0x12, 0x34, 0x56, 0x78]; + let mut response = CtapHid::::busy_error(cid); + let mut expected_packet = [0x00; 64]; + expected_packet[..8].copy_from_slice(&[ + 0x12, + 0x34, + 0x56, + 0x78, + 0xBF, + 0x00, + 0x01, + CtapHidError::ChannelBusy as u8, + ]); + assert_eq!(response.next(), Some(expected_packet)); + assert_eq!(response.next(), None); + } + #[test] fn test_process_single_packet() { let cid = [0x12, 0x34, 0x56, 0x78]; diff --git a/libraries/opensk/src/ctap/mod.rs b/libraries/opensk/src/ctap/mod.rs index f23a1f0e..b457de27 100644 --- a/libraries/opensk/src/ctap/mod.rs +++ b/libraries/opensk/src/ctap/mod.rs @@ -50,7 +50,9 @@ use self::data_formats::{ PublicKeyCredentialDescriptor, PublicKeyCredentialParameter, PublicKeyCredentialSource, PublicKeyCredentialType, PublicKeyCredentialUserEntity, SignatureAlgorithm, }; -use self::hid::{ChannelID, CtapHid, CtapHidCommand, KeepaliveStatus, ProcessedPacket}; +use self::hid::{ + ChannelID, CtapHid, CtapHidCommand, HidPacketIterator, KeepaliveStatus, ProcessedPacket, +}; use self::large_blobs::LargeBlobState; use self::response::{ AuthenticatorGetAssertionResponse, AuthenticatorGetInfoResponse, @@ -61,7 +63,7 @@ use self::status_code::{Ctap2StatusCode, CtapResult}; #[cfg(feature = "with_ctap1")] use self::u2f_up::U2fUserPresenceState; use crate::api::clock::Clock; -use crate::api::connection::{HidConnection, SendOrRecvStatus, UsbEndpoint}; +use crate::api::connection::{HidConnection, RecvStatus, UsbEndpoint}; use crate::api::crypto::ecdsa::{SecretKey as _, Signature}; use crate::api::crypto::hkdf256::Hkdf256; use crate::api::crypto::sha256::Sha256; @@ -149,11 +151,11 @@ pub enum Transport { } impl Transport { - pub fn hid_connection(self, env: &mut E) -> &mut E::HidConnection { + pub fn usb_endpoint(self) -> UsbEndpoint { match self { - Transport::MainHid => env.main_hid_connection(), + Transport::MainHid => UsbEndpoint::MainHid, #[cfg(feature = "vendor_hid")] - Transport::VendorHid => env.vendor_hid_connection(), + Transport::VendorHid => UsbEndpoint::VendorHid, } } } @@ -255,124 +257,91 @@ fn truncate_to_char_boundary(s: &str, mut max: usize) -> &str { } } -// Sends keepalive packet during user presence checking. If user agent replies with CANCEL response, -// returns Err(UserPresenceError::Canceled). -fn send_keepalive_up_needed( +/// Send non-critical packets using fire-and-forget. +fn send_packets( env: &mut E, - channel: Channel, - timeout_ms: usize, -) -> Result<(), UserPresenceError> { + endpoint: UsbEndpoint, + packets: HidPacketIterator, +) -> CtapResult<()> { + for pkt in packets { + env.hid_connection().send(&pkt, endpoint)?; + } + Ok(()) +} + +fn is_cancel(packet: &ProcessedPacket) -> bool { + match packet { + ProcessedPacket::InitPacket { cmd, .. } => *cmd == CtapHidCommand::Cancel as u8, + ProcessedPacket::ContinuationPacket { .. } => false, + } +} + +fn wait_and_respond_busy(env: &mut E, channel: Channel) -> CtapResult<()> { 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_and_maybe_recv(&mut pkt, timeout_ms) { - Ok(SendOrRecvStatus::Timeout) => { - debug_ctap!(env, "Sending a KEEPALIVE packet timed out"); - // The client is likely unresponsive, but let's retry. - } - Err(_) => panic!("Error sending KEEPALIVE packet"), - Ok(SendOrRecvStatus::Sent) => { - debug_ctap!(env, "Sent KEEPALIVE packet"); - } - Ok(SendOrRecvStatus::Received(endpoint)) => { - let rx_transport = match endpoint { - UsbEndpoint::MainHid => Transport::MainHid, - #[cfg(feature = "vendor_hid")] - UsbEndpoint::VendorHid => Transport::VendorHid, - }; - if rx_transport != transport { - debug_ctap!( - env, - "Received a packet on transport {:?} while sending a KEEPALIVE packet on transport {:?}", - rx_transport, transport - ); - // Ignore this packet. - // TODO(liamjm): Support receiving packets on both interfaces. - continue; - } - - // 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", - ); - } - } + let endpoint = transport.usb_endpoint(); + + let mut packet = [0; 64]; + let (up_status, recv_status) = env + .user_presence() + .wait_with_timeout(&mut packet, KEEPALIVE_DELAY_MS)?; + match recv_status { + RecvStatus::Timeout => (), + RecvStatus::Received(rx_endpoint) => { + let (received_cid, processed_packet) = CtapHid::::process_single_packet(&packet); + if rx_endpoint != endpoint || received_cid != cid { + debug_ctap!( + env, + "Received an unrelated packet on endpoint {:?} while sending a KEEPALIVE packet", + rx_endpoint, + ); + let busy_error = CtapHid::::busy_error(received_cid); + // Don't send errors from other channels on the active channel. + let _ = send_packets(env, rx_endpoint, busy_error); + } else if is_cancel(&processed_packet) { + // Ignored, CANCEL specification says: "the authenticator MUST NOT reply" + debug_ctap!(env, "User presence check cancelled"); + return Err(UserPresenceError::Canceled.into()); + } else { + // Ignored. A client shouldn't try to talk to us on this channel yet. + debug_ctap!(env, "Discarded packet while checking user presence."); } } } - Ok(()) + if matches!(up_status, Err(UserPresenceError::Timeout)) { + let keepalive_msg = CtapHid::::keepalive(cid, KeepaliveStatus::UpNeeded); + send_packets(env, endpoint, keepalive_msg)?; + } + up_status.map_err(|e| e.into()) } /// Blocks for user presence. /// /// Returns an error in case of timeout, user declining presence request, or keepalive error. pub fn check_user_presence(env: &mut E, channel: Channel) -> CtapResult<()> { - env.user_presence().check_init(); + const TIMEOUT_ERROR: Ctap2StatusCode = Ctap2StatusCode::CTAP2_ERR_USER_ACTION_TIMEOUT; - // The timeout is N times the keepalive delay. - const TIMEOUT_ITERATIONS: usize = TOUCH_TIMEOUT_MS / KEEPALIVE_DELAY_MS; - - // 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 { 0 } else { KEEPALIVE_DELAY_MS }); - 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. - let keepalive_result = send_keepalive_up_needed(env, channel, KEEPALIVE_DELAY_MS); - if keepalive_result.is_err() { - debug_ctap!( - env, - "Sending keepalive failed with error {:?}", - keepalive_result.as_ref().unwrap_err() - ); - result = keepalive_result; - break; + env.user_presence().check_init(); + let loop_timer = env.clock().make_timer(TOUCH_TIMEOUT_MS); + + // We don't use the '?' operator to always reach check_complete(...). + let mut result = Err(TIMEOUT_ERROR); + while !env.clock().is_elapsed(&loop_timer) { + match wait_and_respond_busy(env, channel) { + Err(TIMEOUT_ERROR) => (), + r => { + result = r; + // We want to break on Ok(()) too, indicating touch. + break; + } } } env.user_presence().check_complete(); - result.map_err(|e| e.into()) + result } /// Holds data necessary to sign an assertion for a credential. @@ -1463,7 +1432,6 @@ mod test { use crate::api::crypto::ecdh::SecretKey as _; use crate::api::customization; use crate::api::key_store::CBOR_CREDENTIAL_ID_SIZE; - use crate::api::user_presence::UserPresenceResult; use crate::ctap::command::AuthenticatorLargeBlobsParameters; use crate::env::test::TestEnv; use crate::env::EcdhSk; @@ -3381,13 +3349,12 @@ mod test { #[test] fn test_check_user_presence_timeout() { - // This will always return timeout. - fn user_presence_timeout() -> UserPresenceResult { - Err(UserPresenceError::Timeout) - } - let mut env = TestEnv::default(); - env.user_presence().set(user_presence_timeout); + let clock = env.clock().clone(); + env.user_presence().set(move || { + clock.advance(100); + Err(UserPresenceError::Timeout) + }); let response = check_user_presence(&mut env, DUMMY_CHANNEL); assert!(matches!( response, diff --git a/libraries/opensk/src/ctap/status_code.rs b/libraries/opensk/src/ctap/status_code.rs index 2e47bcdd..27126180 100644 --- a/libraries/opensk/src/ctap/status_code.rs +++ b/libraries/opensk/src/ctap/status_code.rs @@ -93,7 +93,6 @@ impl From for Ctap2StatusCode { UserPresenceError::Timeout => Self::CTAP2_ERR_USER_ACTION_TIMEOUT, UserPresenceError::Declined => Self::CTAP2_ERR_OPERATION_DENIED, UserPresenceError::Canceled => Self::CTAP2_ERR_KEEPALIVE_CANCEL, - UserPresenceError::Fail => Self::CTAP2_ERR_VENDOR_HARDWARE_FAILURE, } } } diff --git a/libraries/opensk/src/env/mod.rs b/libraries/opensk/src/env/mod.rs index 427450fa..18edfd4a 100644 --- a/libraries/opensk/src/env/mod.rs +++ b/libraries/opensk/src/env/mod.rs @@ -66,12 +66,8 @@ pub trait Env { 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; + /// I/O connection for sending HID packets. + fn hid_connection(&mut self) -> &mut Self::HidConnection; /// Indicates that the last power cycle was not caused by user action. fn boots_after_soft_reset(&self) -> bool; diff --git a/libraries/opensk/src/env/test/mod.rs b/libraries/opensk/src/env/test/mod.rs index 2d9f6119..75b072db 100644 --- a/libraries/opensk/src/env/test/mod.rs +++ b/libraries/opensk/src/env/test/mod.rs @@ -13,19 +13,20 @@ // limitations under the License. use crate::api::clock::Clock; -use crate::api::connection::{HidConnection, SendOrRecvResult, SendOrRecvStatus}; +use crate::api::connection::{HidConnection, RecvStatus, UsbEndpoint}; use crate::api::crypto::software_crypto::SoftwareCrypto; use crate::api::customization::DEFAULT_CUSTOMIZATION; use crate::api::key_store; use crate::api::persist::{Persist, PersistIter}; use crate::api::rng::Rng; -use crate::api::user_presence::{UserPresence, UserPresenceResult}; +use crate::api::user_presence::{UserPresence, UserPresenceResult, UserPresenceWaitResult}; use crate::ctap::status_code::CtapResult; use crate::env::Env; use customization::TestCustomization; use persistent_store::{BufferOptions, BufferStorage, Store}; use rand::rngs::StdRng; use rand::SeedableRng; +use std::sync::{Arc, Mutex}; pub mod customization; @@ -47,15 +48,20 @@ pub struct TestTimer { end_ms: usize, } -#[derive(Debug, Default)] +#[derive(Clone, Debug, Default)] pub struct TestClock { /// The current time, as advanced, in milliseconds. - now_ms: usize, + now_ms: Arc>, } impl TestClock { - pub fn advance(&mut self, milliseconds: usize) { - self.now_ms += milliseconds; + pub fn now(&self) -> usize { + *self.now_ms.lock().unwrap() + } + + pub fn advance(&self, milliseconds: usize) { + let mut locked_now_ms = self.now_ms.lock().unwrap(); + *locked_now_ms += milliseconds; } } @@ -64,18 +70,18 @@ impl Clock for TestClock { fn make_timer(&mut self, milliseconds: usize) -> Self::Timer { TestTimer { - end_ms: self.now_ms + milliseconds, + end_ms: self.now() + milliseconds, } } fn is_elapsed(&mut self, timer: &Self::Timer) -> bool { - self.now_ms >= timer.end_ms + self.now() >= timer.end_ms } #[cfg(feature = "debug_ctap")] fn timestamp_us(&mut self) -> usize { // Unused, but let's implement something because it's easy. - self.now_ms * 1000 + self.now() * 1000 } } @@ -128,9 +134,14 @@ impl Persist for TestEnv { } impl HidConnection for TestEnv { - fn send_and_maybe_recv(&mut self, _buf: &mut [u8; 64], _timeout_ms: usize) -> SendOrRecvResult { - // TODO: Implement I/O from canned requests/responses for integration testing. - Ok(SendOrRecvStatus::Sent) + // TODO: Implement I/O from canned requests/responses for integration testing. + + fn send(&mut self, _buf: &[u8; 64], _endpoint: UsbEndpoint) -> CtapResult<()> { + Ok(()) + } + + fn recv(&mut self, _buf: &mut [u8; 64], _timeout_ms: usize) -> CtapResult { + Ok(RecvStatus::Received(UsbEndpoint::MainHid)) } } @@ -177,8 +188,12 @@ impl TestUserPresence { impl UserPresence for TestUserPresence { fn check_init(&mut self) {} - fn wait_with_timeout(&mut self, _timeout_ms: usize) -> UserPresenceResult { - (self.check)() + fn wait_with_timeout( + &mut self, + _buf: &mut [u8; 64], + _timeout_ms: usize, + ) -> UserPresenceWaitResult { + Ok(((self.check)(), RecvStatus::Timeout)) } fn check_complete(&mut self) {} } @@ -224,12 +239,7 @@ impl Env for TestEnv { &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 { + fn hid_connection(&mut self) -> &mut Self { self } diff --git a/patches/tock/03-add-ctap-modules.patch b/patches/tock/03-add-ctap-modules.patch index d4adb6e9..f4d41350 100644 --- a/patches/tock/03-add-ctap-modules.patch +++ b/patches/tock/03-add-ctap-modules.patch @@ -559,10 +559,10 @@ index 000000000..30cac1323 +} diff --git a/capsules/src/usb/usbc_ctap_hid.rs b/capsules/src/usb/usbc_ctap_hid.rs new file mode 100644 -index 000000000..5ad2c44b3 +index 000000000..fdf92a28e --- /dev/null +++ b/capsules/src/usb/usbc_ctap_hid.rs -@@ -0,0 +1,554 @@ +@@ -0,0 +1,553 @@ +//! A USB HID client of the USB hardware interface + +use super::app::App; @@ -927,9 +927,8 @@ index 000000000..5ad2c44b3 + if self + .client + .map_or(false, |client| client.can_receive_packet(&app)) ++ && self.pending_out.take() + { -+ assert!(self.pending_out.take()); -+ + // Clear any pending packet on the transmitting side. + // It's up to the client to handle the received packet and decide if this packet + // should be re-transmitted or not. diff --git a/src/env/tock/mod.rs b/src/env/tock/mod.rs index 6ebc5778..edb4b50c 100644 --- a/src/env/tock/mod.rs +++ b/src/env/tock/mod.rs @@ -15,36 +15,33 @@ use alloc::boxed::Box; use alloc::vec::Vec; use clock::TockClock; -use core::cell::Cell; use core::convert::TryFrom; use core::marker::PhantomData; +use core::mem; #[cfg(all(target_has_atomic = "8", not(feature = "std")))] use core::sync::atomic::{AtomicBool, Ordering}; -use libtock_buttons::{ButtonListener, ButtonState, Buttons}; use libtock_console::{Console, ConsoleWriter}; -use libtock_drivers::result::{FlexUnwrap, TockError}; use libtock_drivers::timer::Duration; use libtock_drivers::usb_ctap_hid::UsbCtapHid; -use libtock_drivers::{rng, timer, usb_ctap_hid}; +use libtock_drivers::{rng, usb_ctap_hid}; use libtock_leds::Leds; use libtock_platform as platform; -use libtock_platform::{ErrorCode, Syscalls}; -use opensk::api::connection::{ - HidConnection, SendOrRecvError, SendOrRecvResult, SendOrRecvStatus, UsbEndpoint, -}; +use libtock_platform::Syscalls; +use opensk::api::clock::Clock; +use opensk::api::connection::{HidConnection, RecvStatus, UsbEndpoint}; use opensk::api::crypto::software_crypto::SoftwareCrypto; use opensk::api::customization::{CustomizationImpl, AAGUID_LENGTH, DEFAULT_CUSTOMIZATION}; use opensk::api::key_store; use opensk::api::persist::{Persist, PersistIter}; use opensk::api::rng::Rng; -use opensk::api::user_presence::{UserPresence, UserPresenceError, UserPresenceResult}; -use opensk::ctap::status_code::CtapResult; +use opensk::api::user_presence::{UserPresence, UserPresenceError, UserPresenceWaitResult}; +use opensk::ctap::status_code::{Ctap2StatusCode, CtapResult}; use opensk::ctap::Channel; use opensk::env::Env; #[cfg(feature = "std")] use persistent_store::BufferOptions; use persistent_store::{StorageResult, Store}; -use platform::{share, DefaultConfig, Subscribe}; +use platform::DefaultConfig; use rand_core::{impls, CryptoRng, Error, RngCore}; #[cfg(feature = "std")] @@ -76,6 +73,9 @@ const TOCK_CUSTOMIZATION: CustomizationImpl = CustomizationImpl { ..DEFAULT_CUSTOMIZATION }; +// This timeout should rarely be relevant, execution returns without blocking. +const SEND_TIMEOUT_MS: Duration = Duration::from_ms(1000); + /// RNG backed by the TockOS rng driver. pub struct TockRng { _syscalls: PhantomData, @@ -112,28 +112,6 @@ impl RngCore for TockRng { impl Rng for TockRng {} -pub struct TockHidConnection { - endpoint: UsbEndpoint, - s: PhantomData, -} - -impl HidConnection for TockHidConnection { - fn send_and_maybe_recv(&mut self, buf: &mut [u8; 64], timeout_ms: usize) -> SendOrRecvResult { - match UsbCtapHid::::send_or_recv_with_timeout( - buf, - Duration::from_ms(timeout_ms as isize), - self.endpoint as u32, - ) { - 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)) => { - UsbEndpoint::try_from(recv_endpoint as usize).map(SendOrRecvStatus::Received) - } - _ => Err(SendOrRecvError), - } - } -} - pub struct TockEnv< S: Syscalls, C: platform::subscribe::Config + platform::allow_ro::Config = DefaultConfig, @@ -141,10 +119,8 @@ pub struct TockEnv< rng: TockRng, store: Store>, upgrade_storage: Option>, - main_connection: TockHidConnection, - #[cfg(feature = "vendor_hid")] - vendor_connection: TockHidConnection, blink_pattern: usize, + blink_timer: as Clock>::Timer, clock: TockClock, c: PhantomData, } @@ -167,16 +143,8 @@ impl D rng, store, upgrade_storage, - main_connection: TockHidConnection { - endpoint: UsbEndpoint::MainHid, - s: PhantomData, - }, - #[cfg(feature = "vendor_hid")] - vendor_connection: TockHidConnection { - endpoint: UsbEndpoint::VendorHid, - s: PhantomData, - }, blink_pattern: 0, + blink_timer: as Clock>::Timer::default(), clock: TockClock::default(), c: PhantomData, } @@ -287,6 +255,36 @@ where } } +impl HidConnection for TockEnv +where + S: Syscalls, + C: platform::subscribe::Config + platform::allow_ro::Config, +{ + fn send(&mut self, buf: &[u8; 64], endpoint: UsbEndpoint) -> CtapResult<()> { + match UsbCtapHid::::send(buf, SEND_TIMEOUT_MS, endpoint as u32) { + Ok(usb_ctap_hid::SendOrRecvStatus::Timeout) => Err(Ctap2StatusCode::CTAP1_ERR_TIMEOUT), + Ok(usb_ctap_hid::SendOrRecvStatus::Sent) => Ok(()), + Ok(usb_ctap_hid::SendOrRecvStatus::Received(_)) => { + panic!("Returned Received status on send") + } + Err(_) => Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_HARDWARE_FAILURE), + } + } + + fn recv(&mut self, buf: &mut [u8; 64], timeout_ms: usize) -> CtapResult { + match UsbCtapHid::::recv_with_timeout(buf, Duration::from_ms(timeout_ms as isize)) { + Ok(usb_ctap_hid::SendOrRecvStatus::Timeout) => Ok(RecvStatus::Timeout), + Ok(usb_ctap_hid::SendOrRecvStatus::Sent) => { + panic!("Returned Sent status on receive") + } + Ok(usb_ctap_hid::SendOrRecvStatus::Received(recv_endpoint)) => { + UsbEndpoint::try_from(recv_endpoint as usize).map(RecvStatus::Received) + } + Err(_) => Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_HARDWARE_FAILURE), + } + } +} + impl UserPresence for TockEnv where S: Syscalls, @@ -296,93 +294,46 @@ where self.blink_pattern = 0; } - fn wait_with_timeout(&mut self, timeout_ms: usize) -> UserPresenceResult { - if timeout_ms == 0 { - return Err(UserPresenceError::Timeout); + fn wait_with_timeout( + &mut self, + packet: &mut [u8; 64], + timeout_ms: usize, + ) -> UserPresenceWaitResult { + let mut new_timer = self.clock.make_timer(timeout_ms); + mem::swap(&mut self.blink_timer, &mut new_timer); + if self.clock().is_elapsed(&new_timer) { + blink_leds::(self.blink_pattern); + self.blink_pattern += 1; + } else { + mem::swap(&mut self.blink_timer, &mut new_timer); } - blink_leds::(self.blink_pattern); - self.blink_pattern += 1; - - // enable interrupts for all buttons - let num_buttons = Buttons::::count().map_err(|_| UserPresenceError::Fail)?; - (0..num_buttons) - .try_for_each(|n| Buttons::::enable_interrupts(n)) - .map_err(|_| UserPresenceError::Fail)?; - - let button_touched = Cell::new(false); - let button_listener = ButtonListener(|_button_num, state| { - match state { - ButtonState::Pressed => button_touched.set(true), - ButtonState::Released => (), - }; - }); - - // Setup a keep-alive callback but don't enable it yet - let keepalive_expired = Cell::new(false); - let mut keepalive_callback = - timer::with_callback::(|_| keepalive_expired.set(true)); - share::scope::< - ( - Subscribe<_, { libtock_buttons::DRIVER_NUM }, 0>, - Subscribe< - S, - { libtock_drivers::timer::DRIVER_NUM }, - { libtock_drivers::timer::subscribe::CALLBACK }, - >, - ), - _, - _, - >(|handle| { - let (sub_button, sub_timer) = handle.split(); - Buttons::::register_listener(&button_listener, sub_button) - .map_err(|_| UserPresenceError::Fail)?; - - let mut keepalive = keepalive_callback.init().flex_unwrap(); - keepalive_callback - .enable(sub_timer) - .map_err(|_| UserPresenceError::Fail)?; - keepalive - .set_alarm(timer::Duration::from_ms(timeout_ms as isize)) - .flex_unwrap(); - - // Wait for a button touch or an alarm. - libtock_drivers::util::Util::::yieldk_for(|| { - button_touched.get() || keepalive_expired.get() - }); - - Buttons::::unregister_listener(); - - // disable event interrupts for all buttons - (0..num_buttons) - .try_for_each(|n| Buttons::::disable_interrupts(n)) - .map_err(|_| UserPresenceError::Fail)?; - - // Cleanup alarm callback. - match keepalive.stop_alarm() { - Ok(()) => (), - Err(TockError::Command(ErrorCode::Already)) => 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: "); - } - } - - Ok::<(), UserPresenceError>(()) - })?; - if button_touched.get() { + let result = + UsbCtapHid::::recv_with_buttons(packet, Duration::from_ms(timeout_ms as isize)); + let (status, button_touched) = match result { + Ok((status, button_touched)) => (status, button_touched), + Err(_) => return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_HARDWARE_FAILURE), + }; + let recv_status = match status { + usb_ctap_hid::SendOrRecvStatus::Timeout => RecvStatus::Timeout, + usb_ctap_hid::SendOrRecvStatus::Sent => { + panic!("Returned Sent status on receive") + } + usb_ctap_hid::SendOrRecvStatus::Received(recv_endpoint) => { + RecvStatus::Received(UsbEndpoint::try_from(recv_endpoint as usize)?) + } + }; + let up_result = if button_touched { Ok(()) - } else if keepalive_expired.get() { - Err(UserPresenceError::Timeout) } else { - panic!("Unexpected exit condition"); - } + Err(UserPresenceError::Timeout) + }; + Ok((up_result, recv_status)) } fn check_complete(&mut self) { switch_off_leds::(); + self.blink_timer = as Clock>::Timer::default(); } } @@ -403,7 +354,7 @@ impl E type Clock = TockClock; type Write = ConsoleWriter; type Customization = CustomizationImpl; - type HidConnection = TockHidConnection; + type HidConnection = Self; type Crypto = SoftwareCrypto; fn rng(&mut self) -> &mut Self::Rng { @@ -434,13 +385,8 @@ impl E &TOCK_CUSTOMIZATION } - fn main_hid_connection(&mut self) -> &mut Self::HidConnection { - &mut self.main_connection - } - - #[cfg(feature = "vendor_hid")] - fn vendor_hid_connection(&mut self) -> &mut Self::HidConnection { - &mut self.vendor_connection + fn hid_connection(&mut self) -> &mut Self { + self } fn process_vendor_command(&mut self, bytes: &[u8], channel: Channel) -> Option> { diff --git a/src/main.rs b/src/main.rs index a71c8ff9..069ee580 100644 --- a/src/main.rs +++ b/src/main.rs @@ -34,16 +34,15 @@ use libtock_buttons::Buttons; use libtock_console::Console; #[cfg(feature = "debug_ctap")] use libtock_console::ConsoleWriter; -use libtock_drivers::result::FlexUnwrap; -use libtock_drivers::timer::Duration; use libtock_drivers::usb_ctap_hid; #[cfg(not(feature = "std"))] use libtock_runtime::{set_main, stack_size, TockSyscalls}; #[cfg(feature = "std")] use libtock_unittest::fake; use opensk::api::clock::Clock; -use opensk::api::connection::UsbEndpoint; +use opensk::api::connection::{HidConnection, RecvStatus, UsbEndpoint}; use opensk::ctap::hid::HidPacketIterator; +use opensk::ctap::status_code::Ctap2StatusCode; use opensk::ctap::KEEPALIVE_DELAY_MS; use opensk::env::Env; use opensk::Transport; @@ -53,9 +52,6 @@ stack_size! {0x4000} #[cfg(not(feature = "std"))] set_main! {main} -const SEND_TIMEOUT_MS: Duration = Duration::from_ms(1000); -const KEEPALIVE_DELAY_MS_TOCK: Duration = Duration::from_ms(KEEPALIVE_DELAY_MS as isize); - #[cfg(not(feature = "vendor_hid"))] const NUM_ENDPOINTS: usize = 1; #[cfg(feature = "vendor_hid")] @@ -160,14 +156,19 @@ fn main() { let mut pkt_request = [0; 64]; if let Some(packet) = replies.next_packet() { - match usb_ctap_hid::UsbCtapHid::::send( - &packet.packet, - SEND_TIMEOUT_MS, - packet.endpoint as u32, - ) - .flex_unwrap() - { - usb_ctap_hid::SendOrRecvStatus::Sent => { + let hid_connection = ctap.env().hid_connection(); + match hid_connection.send(&packet.packet, packet.endpoint) { + Err(Ctap2StatusCode::CTAP1_ERR_TIMEOUT) => { + #[cfg(feature = "debug_ctap")] + print_packet_notice::( + "Timeout on USB send", + ctap.env().clock().timestamp_us(), + &mut writer, + ); + // The client is unresponsive, so we discard all pending packets. + replies.clear(packet.endpoint); + } + Ok(()) => { #[cfg(feature = "debug_ctap")] print_packet_notice::( "Sent packet", @@ -175,40 +176,23 @@ fn main() { &mut writer, ); } - usb_ctap_hid::SendOrRecvStatus::Timeout => { + Err(_) => panic!("Error on USB send"), + } + } else { + let hid_connection = ctap.env().hid_connection(); + usb_endpoint = match hid_connection.recv(&mut pkt_request, KEEPALIVE_DELAY_MS) { + Ok(RecvStatus::Timeout) => None, + Ok(RecvStatus::Received(endpoint)) => { #[cfg(feature = "debug_ctap")] print_packet_notice::( - "Timeout while sending packet", + "Received packet", ctap.env().clock().timestamp_us(), &mut writer, ); - // The client is unresponsive, so we discard all pending packets. - replies.clear(packet.endpoint); + UsbEndpoint::try_from(endpoint as usize).ok() } - _ => panic!("Unexpected status on USB transmission"), + Err(_) => panic!("Error on USB recv"), }; - } else { - usb_endpoint = - match usb_ctap_hid::UsbCtapHid::::recv_with_timeout( - &mut pkt_request, - KEEPALIVE_DELAY_MS_TOCK, - ) - .flex_unwrap() - { - usb_ctap_hid::SendOrRecvStatus::Received(endpoint) => { - #[cfg(feature = "debug_ctap")] - print_packet_notice::( - "Received packet", - ctap.env().clock().timestamp_us(), - &mut writer, - ); - UsbEndpoint::try_from(endpoint as usize).ok() - } - usb_ctap_hid::SendOrRecvStatus::Sent => { - panic!("Returned transmit status on receive") - } - usb_ctap_hid::SendOrRecvStatus::Timeout => None, - }; } #[cfg(feature = "with_ctap1")] diff --git a/third_party/libtock-drivers/Cargo.toml b/third_party/libtock-drivers/Cargo.toml index 3b3841fa..27237e98 100644 --- a/third_party/libtock-drivers/Cargo.toml +++ b/third_party/libtock-drivers/Cargo.toml @@ -10,6 +10,7 @@ edition = "2018" [dependencies] libtock_alarm = { path = "../../third_party/libtock-rs/apis/alarm" } +libtock_buttons = { path = "../../third_party/libtock-rs/apis/buttons" } libtock_console = { path = "../../third_party/libtock-rs/apis/console" } libtock_platform = { path = "../../third_party/libtock-rs/platform" } diff --git a/third_party/libtock-drivers/src/usb_ctap_hid.rs b/third_party/libtock-drivers/src/usb_ctap_hid.rs index 0faaefad..edd09a58 100644 --- a/third_party/libtock-drivers/src/usb_ctap_hid.rs +++ b/third_party/libtock-drivers/src/usb_ctap_hid.rs @@ -26,6 +26,7 @@ use libtock_platform::{share, DefaultConfig, ErrorCode, Syscalls}; use platform::share::Handle; use platform::subscribe::OneId; use platform::{AllowRo, AllowRw, Subscribe, Upcall}; +use libtock_buttons::{ButtonListener, ButtonState, Buttons}; const DRIVER_NUMBER: u32 = 0x20009; @@ -35,7 +36,7 @@ mod command_nr { pub const CONNECT: u32 = 1; pub const TRANSMIT: u32 = 2; pub const RECEIVE: u32 = 3; - pub const TRANSMIT_OR_RECEIVE: u32 = 4; + pub const _TRANSMIT_OR_RECEIVE: u32 = 4; pub const CANCEL: u32 = 5; } @@ -169,49 +170,6 @@ impl UsbCtapHid { Self::send_detail(buf, timeout_delay, endpoint) } - /// Either sends or receives a packet within a given time. - /// - /// Because USB transactions are initiated by the host, we don't decide whether an IN transaction - /// (send for us), an OUT transaction (receive for us), or no transaction at all will happen next. - /// - /// - If an IN transaction happens first, the initial content of buf is sent to the host and the - /// Sent status is returned. - /// - If an OUT transaction happens first, the content of buf is replaced by the packet received - /// from the host and Received status is returned. In that case, the original content of buf is not - /// sent to the host, and it's up to the caller to retry sending or to handle the packet received - /// from the host. - /// If the timeout elapses, return None. - #[allow(clippy::let_and_return)] - pub fn send_or_recv_with_timeout( - buf: &mut [u8; 64], - timeout_delay: Duration, - endpoint: u32, - ) -> TockResult { - #[cfg(feature = "verbose_usb")] - writeln!( - Console::::writer(), - "Sending packet with timeout of {} ms = {:02x?}", - timeout_delay.ms(), - buf as &[u8] - ) - .unwrap(); - - let result = Self::send_or_recv_with_timeout_detail(buf, timeout_delay, endpoint); - - #[cfg(feature = "verbose_usb")] - if let Ok(SendOrRecvStatus::Received(received_endpoint)) = result { - writeln!( - Console::::writer(), - "Received packet = {:02x?} on endpoint {}", - buf as &[u8], - received_endpoint as u8, - ) - .unwrap(); - } - - result - } - fn recv_with_timeout_detail( buf: &mut [u8; 64], timeout_delay: Duration, @@ -393,7 +351,7 @@ impl UsbCtapHid { // The app should wait for it, but it may never happen if the remote app crashes. // We just return to avoid a deadlock. #[cfg(feature = "debug_ctap")] - writeln!(Console::::writer(), "Couldn't cancel the USB receive").unwrap(); + writeln!(Console::::writer(), "Couldn't cancel the USB send").unwrap(); } Err(e) => panic!("Unexpected error when cancelling USB receive: {:?}", e), } @@ -402,61 +360,69 @@ impl UsbCtapHid { status } - fn send_or_recv_with_timeout_detail( + pub fn recv_with_buttons( buf: &mut [u8; 64], timeout_delay: Duration, - endpoint: u32, - ) -> TockResult { + ) -> TockResult<(SendOrRecvStatus, bool)> { let status: Cell> = Cell::new(None); - let alarm = UsbCtapHidListener(|direction, endpoint| { - let option = match direction { - subscribe_nr::TRANSMIT => Some(SendOrRecvStatus::Sent), - subscribe_nr::RECEIVE => Some(SendOrRecvStatus::Received(endpoint)), - _ => None, - }; - status.set(option); + + let alarm = UsbCtapHidListener(|direction, endpoint| match direction { + subscribe_nr::RECEIVE => status.set(Some(SendOrRecvStatus::Received(endpoint))), + // Unknown direction or "transmitted" sent by the kernel + _ => status.set(None), }); - let mut recv_buf = [0; 64]; - // init the time-out callback but don't enable it yet - let mut timeout_callback = timer::with_callback::(|_| { - status.set(Some(SendOrRecvStatus::Timeout)); + let num_buttons = Buttons::::count().map_err(|_| ErrorCode::Fail)?; + (0..num_buttons) + .try_for_each(|n| Buttons::::enable_interrupts(n)) + .map_err(|_| ErrorCode::Fail)?; + + let button_touched = Cell::new(false); + let button_listener = ButtonListener(|_button_num, state| { + match state { + ButtonState::Pressed => { + status.set(Some(SendOrRecvStatus::Timeout)); + button_touched.set(true); + } + ButtonState::Released => (), + }; }); + + let mut timeout_callback = + timer::with_callback::(|_| status.set(Some(SendOrRecvStatus::Timeout))); let status = share::scope::< ( - AllowRo<_, DRIVER_NUMBER, { ro_allow_nr::TRANSMIT }>, AllowRw<_, DRIVER_NUMBER, { rw_allow_nr::RECEIVE }>, - Subscribe<_, DRIVER_NUMBER, { subscribe_nr::TRANSMIT }>, Subscribe<_, DRIVER_NUMBER, { subscribe_nr::RECEIVE }>, - Subscribe<_, { timer::DRIVER_NUM }, { timer::subscribe::CALLBACK }>, + Subscribe, + Subscribe<_, { libtock_buttons::DRIVER_NUM }, 0>, ), _, _, >(|handle| { - let (allow_ro, allow_rw, sub_send, sub_recv, sub_timer) = handle.split(); - - S::allow_ro::(allow_ro, buf)?; - S::allow_rw::(allow_rw, &mut recv_buf)?; + let (allow, subscribe_recv, subscribe_timer, sub_button) = handle.split(); + S::allow_rw::(allow, buf)?; - Self::register_listener::<{ subscribe_nr::TRANSMIT }, _>(&alarm, sub_send)?; - Self::register_listener::<{ subscribe_nr::RECEIVE }, _>(&alarm, sub_recv)?; + Self::register_listener::<{ subscribe_nr::RECEIVE }, _>(&alarm, subscribe_recv)?; + Buttons::::register_listener(&button_listener, sub_button) + .map_err(|_| ErrorCode::Fail)?; let mut timeout = timeout_callback.init()?; - timeout_callback.enable(sub_timer)?; - timeout.set_alarm(timeout_delay)?; - - // Trigger USB transmission. - S::command( - DRIVER_NUMBER, - command_nr::TRANSMIT_OR_RECEIVE, - endpoint as u32, - 0, - ) - .to_result::<(), ErrorCode>()?; + timeout_callback.enable(subscribe_timer)?; + timeout + .set_alarm(timeout_delay) + .map_err(|_| ErrorCode::Fail)?; - util::Util::::yieldk_for(|| status.get().is_some()); - Self::unregister_listener(subscribe_nr::TRANSMIT); + S::command(DRIVER_NUMBER, command_nr::RECEIVE, 0, 0).to_result::<(), ErrorCode>()?; + + Util::::yieldk_for(|| button_touched.get() || status.get().is_some()); Self::unregister_listener(subscribe_nr::RECEIVE); + Buttons::::unregister_listener(); + + // disable event interrupts for all buttons + (0..num_buttons) + .try_for_each(|n| Buttons::::disable_interrupts(n)) + .map_err(|_| ErrorCode::Fail)?; let status = match status.get() { Some(status) => Ok::(status), @@ -465,15 +431,11 @@ impl UsbCtapHid { // Cleanup alarm callback. match timeout.stop_alarm() { - Ok(_) => (), + Ok(()) => (), Err(TockError::Command(ErrorCode::Already)) => { if matches!(status, SendOrRecvStatus::Timeout) { #[cfg(feature = "debug_ctap")] - writeln!( - Console::::writer(), - "The send/receive timeout already expired, but the callback wasn't executed." - ) - .unwrap(); + write!(Console::::writer(), ".").unwrap(); } } Err(_e) => { @@ -491,7 +453,7 @@ impl UsbCtapHid { #[cfg(feature = "verbose_usb")] writeln!( Console::::writer(), - "Cancelling USB transaction due to timeout" + "Cancelling USB receive due to timeout" ) .unwrap(); let result = @@ -505,17 +467,12 @@ impl UsbCtapHid { // The app should wait for it, but it may never happen if the remote app crashes. // We just return to avoid a deadlock. #[cfg(feature = "debug_ctap")] - writeln!(Console::::writer(), "Couldn't cancel the transaction").unwrap(); + writeln!(Console::::writer(), "Couldn't cancel the USB receive with buttons").unwrap(); } - Err(e) => panic!("Unexpected error when cancelling USB transaction: {:?}", e), + Err(e) => panic!("Unexpected error when cancelling USB receive: {:?}", e), } - #[cfg(feature = "debug_ctap")] - writeln!(Console::::writer(), "Cancelled USB transaction!").unwrap(); } - if matches!(status, Ok(SendOrRecvStatus::Received(_))) { - buf.copy_from_slice(&recv_buf); - } - status + status.map(|s| (s, button_touched.get())) } }