Merge pull request #435 from ia0/env_console

Add write to the environment

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<Message> {
+    pub fn parse_packet(
+        &mut self,
+        env: &mut impl Env,
+        packet: &HidPacket,
+        clock_value: ClockValue,
+    ) -> Option<Message> {
         match self
             .assembler
             .parse_packet(packet, Timestamp::<isize>::from_clock_value(clock_value))
         {
             Ok(Some(message)) => {
-                #[cfg(feature = "debug_ctap")]
+                debug_ctap!(env, "Received message: {:02x?}", message);
-                writeln!(&mut Console::new(), "Received message: {:02x?}", message).unwrap();
                 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")]
+            debug_ctap!(env, "Sending message: {:02x?}", processed_message);
-            writeln!(
-                &mut Console::new(),
-                "Sending message: {:02x?}",
-                processed_message
-            )
-            .unwrap();
             CtapHid::split_message(processed_message)
         } else {
             HidPacketIterator::none()

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<u8> {
         let cmd = Command::deserialize(command_cbor);
-        #[cfg(feature = "debug_ctap")]
+        debug_ctap!(env, "Received command: {:#?}", cmd);
-        writeln!(&mut Console::new(), "Received command: {:#?}", cmd).unwrap();
         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")]
+                debug_ctap!(env, "Sending response: {:#?}", response);
-                writeln!(&mut Console::new(), "Sending response: {:#?}", response).unwrap();
                 match response {
                     Ok(response_data) => {
                         let mut response_vec = vec![0x00];

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

src/env/test/mod.rs

@@ -19,6 +19,14 @@ pub struct TestUserPresence {
     check: Box<dyn Fn(ChannelID) -> 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
+    }
 }

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<TockStorage> {
 }
 
 impl UserPresence for TockEnv {
-    fn check(&self, cid: ChannelID) -> Result<(), Ctap2StatusCode> {
+    fn check(&mut self, cid: ChannelID) -> Result<(), Ctap2StatusCode> {
-        check_user_presence(cid)
+        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<isize>,
 ) -> 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")]
+                debug_ctap!(env, "Sending a KEEPALIVE packet timed out");
-                writeln!(Console::new(), "Sending a KEEPALIVE packet timed out").unwrap();
                 // 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")]
+                debug_ctap!(env, "Sent KEEPALIVE packet");
-                writeln!(Console::new(), "Sent KEEPALIVE packet").unwrap();
             }
             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")]
+                    debug_ctap!(
-                    writeln!(
+                        env,
-                        Console::new(),
                         "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")]
+                            debug_ctap!(env, "User presence check cancelled");
-                            writeln!(Console::new(), "User presence check cancelled").unwrap();
                             return Err(Ctap2StatusCode::CTAP2_ERR_KEEPALIVE_CANCEL);
                         } else {
-                            #[cfg(feature = "debug_ctap")]
+                            debug_ctap!(
-                            writeln!(
+                                env,
-                                Console::new(),
                                 "Discarded packet with command {} received while sending a KEEPALIVE packet",
                                 cmd,
-                            )
+                            );
-                            .unwrap();
                         }
                     }
                     ProcessedPacket::ContinuationPacket { .. } => {
-                        #[cfg(feature = "debug_ctap")]
+                        debug_ctap!(
-                        writeln!(
+                            env,
-                            Console::new(),
                             "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<isize> = 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() {

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")]