diff --git a/src/ctap/hid/mod.rs b/src/ctap/hid/mod.rs index f3fa288..568dbce 100644 --- a/src/ctap/hid/mod.rs +++ b/src/ctap/hid/mod.rs @@ -26,10 +26,6 @@ use crate::env::Env; use alloc::vec; use alloc::vec::Vec; use arrayref::{array_ref, array_refs}; -#[cfg(feature = "debug_ctap")] -use core::fmt::Write; -#[cfg(feature = "debug_ctap")] -use libtock_drivers::console::Console; use libtock_drivers::timer::{ClockValue, Duration, Timestamp}; pub type HidPacket = [u8; 64]; @@ -209,14 +205,18 @@ impl CtapHid { /// Ignoring the others is incorrect behavior. You have to at least replace them with an error /// message: /// `CtapHid::error_message(message.cid, CtapHid::ERR_INVALID_CMD)` - pub fn parse_packet(&mut self, packet: &HidPacket, clock_value: ClockValue) -> Option { + pub fn parse_packet( + &mut self, + env: &mut impl Env, + packet: &HidPacket, + clock_value: ClockValue, + ) -> Option { match self .assembler .parse_packet(packet, Timestamp::::from_clock_value(clock_value)) { Ok(Some(message)) => { - #[cfg(feature = "debug_ctap")] - writeln!(&mut Console::new(), "Received message: {:02x?}", message).unwrap(); + debug_ctap!(env, "Received message: {:02x?}", message); self.preprocess_message(message) } Ok(None) => { @@ -392,15 +392,9 @@ impl CtapHid { clock_value: ClockValue, ctap_state: &mut CtapState, ) -> HidPacketIterator { - if let Some(message) = self.parse_packet(packet, clock_value) { + if let Some(message) = self.parse_packet(env, packet, clock_value) { let processed_message = self.process_message(env, message, clock_value, ctap_state); - #[cfg(feature = "debug_ctap")] - writeln!( - &mut Console::new(), - "Sending message: {:02x?}", - processed_message - ) - .unwrap(); + debug_ctap!(env, "Sending message: {:02x?}", processed_message); CtapHid::split_message(processed_message) } else { HidPacketIterator::none() diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index f78aaf1..e4a748c 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -73,15 +73,11 @@ use alloc::vec::Vec; use arrayref::array_ref; use byteorder::{BigEndian, ByteOrder}; use core::convert::TryFrom; -#[cfg(feature = "debug_ctap")] -use core::fmt::Write; use crypto::ecdsa; use crypto::hmac::{hmac_256, verify_hmac_256}; use crypto::rng256::Rng256; use crypto::sha256::Sha256; use crypto::Hash256; -#[cfg(feature = "debug_ctap")] -use libtock_drivers::console::Console; use libtock_drivers::timer::{ClockValue, Duration}; use sk_cbor as cbor; use sk_cbor::cbor_map_options; @@ -455,8 +451,7 @@ impl CtapState { now: ClockValue, ) -> Vec { let cmd = Command::deserialize(command_cbor); - #[cfg(feature = "debug_ctap")] - writeln!(&mut Console::new(), "Received command: {:#?}", cmd).unwrap(); + debug_ctap!(env, "Received command: {:#?}", cmd); match cmd { Ok(command) => { // Correct behavior between CTAP1 and CTAP2 isn't defined yet. Just a guess. @@ -528,8 +523,7 @@ impl CtapState { self.process_vendor_upgrade_info(env) } }; - #[cfg(feature = "debug_ctap")] - writeln!(&mut Console::new(), "Sending response: {:#?}", response).unwrap(); + debug_ctap!(env, "Sending response: {:#?}", response); match response { Ok(response_data) => { let mut response_vec = vec![0x00]; diff --git a/src/env/mod.rs b/src/env/mod.rs index 9db1b70..03d6f92 100644 --- a/src/env/mod.rs +++ b/src/env/mod.rs @@ -13,7 +13,7 @@ pub trait UserPresence { /// Blocks for user presence. /// /// Returns an error in case of timeout or keepalive error. - fn check(&self, cid: ChannelID) -> Result<(), Ctap2StatusCode>; + fn check(&mut self, cid: ChannelID) -> Result<(), Ctap2StatusCode>; } /// Describes what CTAP needs to function. @@ -23,6 +23,7 @@ pub trait Env { type Storage: Storage; type UpgradeStorage: UpgradeStorage; type FirmwareProtection: FirmwareProtection; + type Write: core::fmt::Write; fn rng(&mut self) -> &mut Self::Rng; fn user_presence(&mut self) -> &mut Self::UserPresence; @@ -35,4 +36,12 @@ pub trait Env { fn upgrade_storage(&mut self) -> Option<&mut Self::UpgradeStorage>; fn firmware_protection(&mut self) -> &mut Self::FirmwareProtection; + + /// Creates a write instance for debugging. + /// + /// This API doesn't return a reference such that drop may flush. This matches the Tock + /// environment. Non-Tock embedded environments should use the defmt feature (to be implemented + /// using the defmt crate) and ignore this API. Non-embedded environments may either use this + /// API or use the log feature (to be implemented using the log crate). + fn write(&mut self) -> Self::Write; } diff --git a/src/env/test/mod.rs b/src/env/test/mod.rs index 66eb031..b169a34 100644 --- a/src/env/test/mod.rs +++ b/src/env/test/mod.rs @@ -19,6 +19,14 @@ pub struct TestUserPresence { check: Box Result<(), Ctap2StatusCode>>, } +pub struct TestWrite; + +impl core::fmt::Write for TestWrite { + fn write_str(&mut self, _: &str) -> core::fmt::Result { + Ok(()) + } +} + fn new_storage() -> BufferStorage { // Use the Nordic configuration. const PAGE_SIZE: usize = 0x1000; @@ -63,7 +71,7 @@ impl TestUserPresence { } impl UserPresence for TestUserPresence { - fn check(&self, cid: ChannelID) -> Result<(), Ctap2StatusCode> { + fn check(&mut self, cid: ChannelID) -> Result<(), Ctap2StatusCode> { (self.check)(cid) } } @@ -80,6 +88,7 @@ impl Env for TestEnv { type Storage = BufferStorage; type UpgradeStorage = BufferUpgradeStorage; type FirmwareProtection = Self; + type Write = TestWrite; fn rng(&mut self) -> &mut Self::Rng { &mut self.rng @@ -100,4 +109,8 @@ impl Env for TestEnv { fn firmware_protection(&mut self) -> &mut Self::FirmwareProtection { self } + + fn write(&mut self) -> Self::Write { + TestWrite + } } diff --git a/src/env/tock/mod.rs b/src/env/tock/mod.rs index 8b249cb..cc22838 100644 --- a/src/env/tock/mod.rs +++ b/src/env/tock/mod.rs @@ -4,13 +4,10 @@ use crate::ctap::hid::{ChannelID, CtapHid, CtapHidCommand, KeepaliveStatus, Proc use crate::ctap::status_code::Ctap2StatusCode; use crate::env::{Env, UserPresence}; use core::cell::Cell; -#[cfg(feature = "debug_ctap")] -use core::fmt::Write; use core::sync::atomic::{AtomicBool, Ordering}; use crypto::rng256::TockRng256; use libtock_core::result::{CommandError, EALREADY}; use libtock_drivers::buttons::{self, ButtonState}; -#[cfg(feature = "debug_ctap")] use libtock_drivers::console::Console; use libtock_drivers::result::{FlexUnwrap, TockError}; use libtock_drivers::timer::Duration; @@ -57,8 +54,8 @@ pub fn take_storage() -> StorageResult { } impl UserPresence for TockEnv { - fn check(&self, cid: ChannelID) -> Result<(), Ctap2StatusCode> { - check_user_presence(cid) + fn check(&mut self, cid: ChannelID) -> Result<(), Ctap2StatusCode> { + check_user_presence(self, cid) } } @@ -81,6 +78,7 @@ impl Env for TockEnv { type Storage = TockStorage; type UpgradeStorage = TockUpgradeStorage; type FirmwareProtection = Self; + type Write = Console; fn rng(&mut self) -> &mut Self::Rng { &mut self.rng @@ -101,10 +99,15 @@ impl Env for TockEnv { fn firmware_protection(&mut self) -> &mut Self::FirmwareProtection { self } + + fn write(&mut self) -> Self::Write { + Console::new() + } } // Returns whether the keepalive was sent, or false if cancelled. fn send_keepalive_up_needed( + env: &mut TockEnv, cid: ChannelID, timeout: Duration, ) -> Result<(), Ctap2StatusCode> { @@ -113,52 +116,43 @@ fn send_keepalive_up_needed( let status = usb_ctap_hid::send_or_recv_with_timeout(&mut pkt, timeout); match status { None => { - #[cfg(feature = "debug_ctap")] - writeln!(Console::new(), "Sending a KEEPALIVE packet timed out").unwrap(); + debug_ctap!(env, "Sending a KEEPALIVE packet timed out"); // TODO: abort user presence test? } Some(usb_ctap_hid::SendOrRecvStatus::Error) => panic!("Error sending KEEPALIVE packet"), Some(usb_ctap_hid::SendOrRecvStatus::Sent) => { - #[cfg(feature = "debug_ctap")] - writeln!(Console::new(), "Sent KEEPALIVE packet").unwrap(); + debug_ctap!(env, "Sent KEEPALIVE packet"); } Some(usb_ctap_hid::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 { - #[cfg(feature = "debug_ctap")] - writeln!( - Console::new(), + debug_ctap!( + env, "Received a packet on channel ID {:?} while sending a KEEPALIVE packet", received_cid, - ) - .unwrap(); + ); 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. - #[cfg(feature = "debug_ctap")] - writeln!(Console::new(), "User presence check cancelled").unwrap(); + debug_ctap!(env, "User presence check cancelled"); return Err(Ctap2StatusCode::CTAP2_ERR_KEEPALIVE_CANCEL); } else { - #[cfg(feature = "debug_ctap")] - writeln!( - Console::new(), + debug_ctap!( + env, "Discarded packet with command {} received while sending a KEEPALIVE packet", cmd, - ) - .unwrap(); + ); } } ProcessedPacket::ContinuationPacket { .. } => { - #[cfg(feature = "debug_ctap")] - writeln!( - Console::new(), + debug_ctap!( + env, "Discarded continuation packet received while sending a KEEPALIVE packet", - ) - .unwrap(); + ); } } } @@ -219,13 +213,13 @@ pub fn switch_off_leds() { const KEEPALIVE_DELAY_MS: isize = 100; pub const KEEPALIVE_DELAY: Duration = Duration::from_ms(KEEPALIVE_DELAY_MS); -fn check_user_presence(cid: ChannelID) -> Result<(), Ctap2StatusCode> { +fn check_user_presence(env: &mut TockEnv, cid: ChannelID) -> 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(cid, KEEPALIVE_DELAY)?; + send_keepalive_up_needed(env, cid, KEEPALIVE_DELAY)?; // Listen to the button presses. let button_touched = Cell::new(false); @@ -275,7 +269,7 @@ fn check_user_presence(cid: ChannelID) -> Result<(), Ctap2StatusCode> { // 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(cid, KEEPALIVE_DELAY); + keepalive_response = send_keepalive_up_needed(env, cid, KEEPALIVE_DELAY); } if button_touched.get() || keepalive_response.is_err() { diff --git a/src/lib.rs b/src/lib.rs index 26573c2..d16ce8d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,6 +24,23 @@ use crate::ctap::CtapState; use crate::env::Env; use libtock_drivers::timer::ClockValue; +// Those macros should eventually be split into trace, debug, info, warn, and error macros when +// adding either the defmt or log feature and crate dependency. +#[cfg(feature = "debug_ctap")] +macro_rules! debug_ctap { + ($env: expr, $($rest:tt)*) => { + use core::fmt::Write; + writeln!($env.write(), $($rest)*).unwrap(); + }; +} +#[cfg(not(feature = "debug_ctap"))] +macro_rules! debug_ctap { + ($env: expr, $($rest:tt)*) => { + // To avoid unused variable warnings. + let _ = $env; + }; +} + pub mod api; // Implementation details must be public for testing (in particular fuzzing). #[cfg(feature = "std")]