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..f66f4ac2 100644 --- a/libraries/opensk/src/api/connection.rs +++ b/libraries/opensk/src/api/connection.rs @@ -46,7 +46,8 @@ 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) -> SendOrRecvResult; + fn recv(&mut self, buf: &mut [u8; 64], timeout_ms: usize) -> SendOrRecvResult; } #[cfg(test)] diff --git a/libraries/opensk/src/api/user_presence.rs b/libraries/opensk/src/api/user_presence.rs index 6b0dbbe6..7f7f24c7 100644 --- a/libraries/opensk/src/api/user_presence.rs +++ b/libraries/opensk/src/api/user_presence.rs @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use crate::api::connection::UsbEndpoint; + #[derive(Debug)] pub enum UserPresenceError { /// User explicitly declined user presence check. @@ -24,7 +26,10 @@ pub enum UserPresenceError { Fail, } -pub type UserPresenceResult = Result<(), UserPresenceError>; +pub type UserPresenceResult = ( + Result<(), UserPresenceError>, + Option<([u8; 64], UsbEndpoint)>, +); pub trait UserPresence { /// Initializes for a user presence check. @@ -35,6 +40,7 @@ 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`]. + /// Additionally returns a packet if one was received during the wait. fn wait_with_timeout(&mut self, timeout_ms: usize) -> UserPresenceResult; /// 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..ac48f32e 100644 --- a/libraries/opensk/src/ctap/hid/mod.rs +++ b/libraries/opensk/src/ctap/hid/mod.rs @@ -440,6 +440,11 @@ impl CtapHid { }) } + /// Generates a 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 9cc430ec..75e8f2d3 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, @@ -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,82 +257,17 @@ 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( - env: &mut E, - channel: Channel, - timeout_ms: usize, -) -> 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_and_maybe_recv(&mut pkt, timeout_ms) { +/// Send non-critical packets using fire-and-forget. +fn send_packets(env: &mut E, endpoint: UsbEndpoint, packets: HidPacketIterator) { + for pkt in packets { + match env.hid_connection().send(&pkt, endpoint) { 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", - ); - } - } + debug_ctap!(env, "Timeout sending packet"); } + Ok(SendOrRecvStatus::Sent) => (), + _ => panic!("Error sending packet"), } } - Ok(()) } /// Blocks for user presence. @@ -338,37 +275,62 @@ fn send_keepalive_up_needed( /// 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(); + let loop_timer = env.clock().make_timer(TOUCH_TIMEOUT_MS); - // The timeout is N times the keepalive delay. - const TIMEOUT_ITERATIONS: usize = TOUCH_TIMEOUT_MS / KEEPALIVE_DELAY_MS; + let (cid, transport) = match channel { + Channel::MainHid(cid) => (cid, Transport::MainHid), + #[cfg(feature = "vendor_hid")] + Channel::VendorHid(cid) => (cid, Transport::VendorHid), + }; + let endpoint = transport.usb_endpoint(); // 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; + while !env.clock().is_elapsed(&loop_timer) { + let (status, data) = env.user_presence().wait_with_timeout(KEEPALIVE_DELAY_MS); + if let Some((packet, rx_endpoint)) = data { + 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); + send_packets(env, rx_endpoint, busy_error); + } + match processed_packet { + ProcessedPacket::InitPacket { cmd, .. } => { + if cmd == CtapHidCommand::Cancel as u8 { + // 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 with command {} received while sending a KEEPALIVE packet", + cmd, + ); + } + } + // Ignored. We likely ignore the init packet before as well. + ProcessedPacket::ContinuationPacket { .. } => { + debug_ctap!( + env, + "Discarded continuation packet received while sending a KEEPALIVE packet", + ); + } + } } - // 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; + if !matches!(status, Err(UserPresenceError::Timeout)) { + result = status; break; } + let keepalive_msg = CtapHid::::keepalive(cid, KeepaliveStatus::UpNeeded); + send_packets(env, endpoint, keepalive_msg); } env.user_presence().check_complete(); @@ -2290,7 +2252,8 @@ mod test { #[test] fn test_process_make_credential_cancelled() { let mut env = TestEnv::default(); - env.user_presence().set(|| Err(UserPresenceError::Canceled)); + env.user_presence() + .set(|| (Err(UserPresenceError::Canceled), None)); let mut ctap_state = CtapState::::new(&mut env); let make_credential_params = create_minimal_make_credential_parameters(); @@ -3170,7 +3133,8 @@ mod test { #[test] fn test_process_reset_cancelled() { let mut env = TestEnv::default(); - env.user_presence().set(|| Err(UserPresenceError::Canceled)); + env.user_presence() + .set(|| (Err(UserPresenceError::Canceled), None)); let mut ctap_state = CtapState::::new(&mut env); let reset_reponse = ctap_state.process_reset(&mut env, DUMMY_CHANNEL); @@ -3396,22 +3360,6 @@ mod test { assert!(matches!(response, Ok(_))); } - #[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 response = check_user_presence(&mut env, DUMMY_CHANNEL); - assert!(matches!( - response, - Err(Ctap2StatusCode::CTAP2_ERR_USER_ACTION_TIMEOUT) - )); - } - #[test] fn test_channel_interleaving() { let mut env = TestEnv::default(); 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..836720b4 100644 --- a/libraries/opensk/src/env/test/mod.rs +++ b/libraries/opensk/src/env/test/mod.rs @@ -13,7 +13,7 @@ // limitations under the License. use crate::api::clock::Clock; -use crate::api::connection::{HidConnection, SendOrRecvResult, SendOrRecvStatus}; +use crate::api::connection::{HidConnection, SendOrRecvResult, SendOrRecvStatus, UsbEndpoint}; use crate::api::crypto::software_crypto::SoftwareCrypto; use crate::api::customization::DEFAULT_CUSTOMIZATION; use crate::api::key_store; @@ -128,17 +128,22 @@ 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. + // TODO: Implement I/O from canned requests/responses for integration testing. + + fn send(&mut self, _buf: &[u8; 64], _endpoint: UsbEndpoint) -> SendOrRecvResult { Ok(SendOrRecvStatus::Sent) } + + fn recv(&mut self, _buf: &mut [u8; 64], _timeout_ms: usize) -> SendOrRecvResult { + Ok(SendOrRecvStatus::Received(UsbEndpoint::MainHid)) + } } impl Default for TestEnv { fn default() -> Self { let rng = StdRng::seed_from_u64(0); let user_presence = TestUserPresence { - check: Box::new(|| Ok(())), + check: Box::new(|| (Ok(()), None)), }; let storage = new_storage(); let store = Store::new(storage).ok().unwrap(); @@ -224,12 +229,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/src/env/tock/mod.rs b/src/env/tock/mod.rs index 6ebc5778..15bbcce3 100644 --- a/src/env/tock/mod.rs +++ b/src/env/tock/mod.rs @@ -15,20 +15,17 @@ use alloc::boxed::Box; use alloc::vec::Vec; use clock::TockClock; -use core::cell::Cell; use core::convert::TryFrom; use core::marker::PhantomData; #[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 libtock_platform::Syscalls; use opensk::api::connection::{ HidConnection, SendOrRecvError, SendOrRecvResult, SendOrRecvStatus, UsbEndpoint, }; @@ -44,7 +41,7 @@ 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,9 +119,6 @@ pub struct TockEnv< rng: TockRng, store: Store>, upgrade_storage: Option>, - main_connection: TockHidConnection, - #[cfg(feature = "vendor_hid")] - vendor_connection: TockHidConnection, blink_pattern: usize, clock: TockClock, c: PhantomData, @@ -167,15 +142,6 @@ 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, clock: TockClock::default(), c: PhantomData, @@ -287,6 +253,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) -> SendOrRecvResult { + match UsbCtapHid::::send(buf, SEND_TIMEOUT_MS, 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(_)) => { + panic!("Returned Received status on send") + } + Err(_) => Err(SendOrRecvError), + } + } + + fn recv(&mut self, buf: &mut [u8; 64], timeout_ms: usize) -> SendOrRecvResult { + match UsbCtapHid::::recv_with_timeout(buf, Duration::from_ms(timeout_ms as isize)) { + Ok(usb_ctap_hid::SendOrRecvStatus::Timeout) => Ok(SendOrRecvStatus::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(SendOrRecvStatus::Received) + } + Err(_) => Err(SendOrRecvError), + } + } +} + impl UserPresence for TockEnv where S: Syscalls, @@ -297,87 +293,32 @@ where } fn wait_with_timeout(&mut self, timeout_ms: usize) -> UserPresenceResult { - if timeout_ms == 0 { - return Err(UserPresenceError::Timeout); - } 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: "); - } + let mut packet = [0; 64]; + let result = + UsbCtapHid::::recv_with_buttons(&mut packet, Duration::from_ms(timeout_ms as isize)); + let (status, button_touched) = match result { + Ok((status, button_touched)) => (status, button_touched), + Err(_) => return (Err(UserPresenceError::Fail), None), + }; + let data = match status { + usb_ctap_hid::SendOrRecvStatus::Timeout => None, + usb_ctap_hid::SendOrRecvStatus::Sent => { + panic!("Returned Sent status on receive") } + usb_ctap_hid::SendOrRecvStatus::Received(recv_endpoint) => { + UsbEndpoint::try_from(recv_endpoint as usize) + .ok() + .map(|e| (packet, e)) + } + }; - Ok::<(), UserPresenceError>(()) - })?; - - if button_touched.get() { - Ok(()) - } else if keepalive_expired.get() { - Err(UserPresenceError::Timeout) + if button_touched { + (Ok(()), data) } else { - panic!("Unexpected exit condition"); + (Err(UserPresenceError::Timeout), data) } } @@ -403,7 +344,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 +375,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..fecb0f56 100644 --- a/src/main.rs +++ b/src/main.rs @@ -34,15 +34,13 @@ 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, SendOrRecvStatus, UsbEndpoint}; use opensk::ctap::hid::HidPacketIterator; use opensk::ctap::KEEPALIVE_DELAY_MS; use opensk::env::Env; @@ -53,9 +51,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 +155,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) { + Ok(SendOrRecvStatus::Timeout) => { + #[cfg(feature = "debug_ctap")] + print_packet_notice::( + "Timeout while sending packet", + ctap.env().clock().timestamp_us(), + &mut writer, + ); + // The client is unresponsive, so we discard all pending packets. + replies.clear(packet.endpoint); + } + Ok(SendOrRecvStatus::Sent) => { #[cfg(feature = "debug_ctap")] print_packet_notice::( "Sent packet", @@ -175,40 +175,23 @@ fn main() { &mut writer, ); } - usb_ctap_hid::SendOrRecvStatus::Timeout => { + _ => panic!("Unexpected status on USB send"), + } + } else { + let hid_connection = ctap.env().hid_connection(); + usb_endpoint = match hid_connection.recv(&mut pkt_request, KEEPALIVE_DELAY_MS) { + Ok(SendOrRecvStatus::Timeout) => None, + Ok(SendOrRecvStatus::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"), + _ => panic!("Unexpected status 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())) } }