Separates HID command logic (#443)

* moves HID logic related to CTAP state out of the HID mod

* fixes license headers

src/ctap/hid/mod.rs

@@ -1,4 +1,4 @@
-// Copyright 2019-2021 Google LLC
+// Copyright 2019-2022 Google LLC
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -22,11 +22,7 @@ pub use self::receive::MessageAssembler;
 use self::receive::MessageAssembler;
 pub use self::send::HidPacketIterator;
 use super::super::clock::{ClockInt, CtapInstant};
-#[cfg(feature = "with_ctap1")]
-use super::ctap1;
 use super::status_code::Ctap2StatusCode;
-use super::timed_permission::TimedPermission;
-use super::CtapState;
 use crate::env::Env;
 use alloc::vec;
 use alloc::vec::Vec;
@@ -144,13 +140,18 @@ pub enum KeepaliveStatus {
 ///
 /// This includes
 /// - state from not fully processed messages,
-/// - all allocated channels,
+/// - all allocated channels.
-/// - information about requested winks.
 ///
-/// The wink information can be polled to decide to i.e. blink LEDs.
+/// To process a packet and receive the response, call `parse_packet`. If you didn't receive any
+/// message or preprocessing discarded it, stop. Else process the message further, by handling the
+/// commands:
 ///
-/// To process a packet and receive the response, you can call `process_hid_packet`.
+/// - PING (optional)
-/// If you want more control, you can also do the processing in steps:
+/// - MSG
+/// - WINK
+/// - CBOR
+///
+/// To get packets to send from your processed message, call `split_message`. Summary:
 ///
 /// 1.  `HidPacket` -> `Option<Message>`
 /// 2.  `Option<Message>` -> `Message`
@@ -161,7 +162,7 @@ pub enum KeepaliveStatus {
 ///
 /// 1.  `parse_packet` assembles the message and preprocesses all pure HID commands and errors.
 /// 2.  If you didn't receive any message or preprocessing discarded it, stop.
-/// 3.  `process_message` handles all protocol interactions.
+/// 3.  Handles all CTAP protocol interactions.
 /// 4.  `split_message` creates packets out of the response message.
 pub struct CtapHid {
     assembler: MessageAssembler,
@@ -172,7 +173,6 @@ pub struct CtapHid {
     // u32::to/from_be_bytes methods).
     // TODO(kaczmarczyck) We might want to limit or timeout open channels.
     allocated_cids: usize,
-    pub(crate) wink_permission: TimedPermission,
 }
 
 impl CtapHid {
@@ -202,13 +202,11 @@ impl CtapHid {
     const CAPABILITIES: u8 = Self::CAPABILITY_WINK | Self::CAPABILITY_CBOR | Self::CAPABILITY_NMSG;
     // TODO: Is this timeout duration specified?
     const TIMEOUT_DURATION: Milliseconds<ClockInt> = Milliseconds(100 as ClockInt);
-    const WINK_TIMEOUT_DURATION: Milliseconds<ClockInt> = Milliseconds(5000 as ClockInt);
 
     pub fn new() -> CtapHid {
         CtapHid {
             assembler: MessageAssembler::new(),
             allocated_cids: 0,
-            wink_permission: TimedPermission::waiting(),
         }
     }
 
@@ -324,81 +322,6 @@ impl CtapHid {
         }
     }
 
-    /// Processes a message's commands that affect the protocol outside HID.
-    pub fn process_message(
-        &mut self,
-        env: &mut impl Env,
-        message: Message,
-        clock_value: CtapInstant,
-        ctap_state: &mut CtapState,
-    ) -> Message {
-        // If another command arrives, stop winking to prevent accidential button touches.
-        self.wink_permission = TimedPermission::waiting();
-
-        let cid = message.cid;
-        match message.cmd {
-            // CTAP 2.1 from 2021-06-15, section 11.2.9.1.1.
-            CtapHidCommand::Msg => {
-                // If we don't have CTAP1 backward compatibilty, this command is invalid.
-                #[cfg(not(feature = "with_ctap1"))]
-                return CtapHid::error_message(cid, CtapHidError::InvalidCmd);
-
-                #[cfg(feature = "with_ctap1")]
-                match ctap1::Ctap1Command::process_command(
-                    env,
-                    &message.payload,
-                    ctap_state,
-                    clock_value,
-                ) {
-                    Ok(payload) => CtapHid::ctap1_success_message(cid, &payload),
-                    Err(ctap1_status_code) => CtapHid::ctap1_error_message(cid, ctap1_status_code),
-                }
-            }
-            // CTAP 2.1 from 2021-06-15, section 11.2.9.1.2.
-            CtapHidCommand::Cbor => {
-                // Each transaction is atomic, so we process the command directly here and
-                // don't handle any other packet in the meantime.
-                // TODO: Send "Processing" type keep-alive packets in the meantime.
-                let response = ctap_state.process_command(env, &message.payload, cid, clock_value);
-                Message {
-                    cid,
-                    cmd: CtapHidCommand::Cbor,
-                    payload: response,
-                }
-            }
-            // CTAP 2.1 from 2021-06-15, section 11.2.9.2.1.
-            CtapHidCommand::Wink => {
-                if message.payload.is_empty() {
-                    self.wink_permission =
-                        TimedPermission::granted(clock_value, CtapHid::WINK_TIMEOUT_DURATION);
-                    // The response is empty like the request.
-                    message
-                } else {
-                    CtapHid::error_message(cid, CtapHidError::InvalidLen)
-                }
-            }
-            // All other commands have already been processed, keep them as is.
-            _ => message,
-        }
-    }
-
-    /// Processes an incoming USB HID packet, and returns an iterator for all outgoing packets.
-    pub fn process_hid_packet(
-        &mut self,
-        env: &mut impl Env,
-        packet: &HidPacket,
-        clock_value: CtapInstant,
-        ctap_state: &mut CtapState,
-    ) -> HidPacketIterator {
-        if let Some(message) = self.parse_packet(env, packet, clock_value) {
-            let processed_message = self.process_message(env, message, clock_value, ctap_state);
-            debug_ctap!(env, "Sending message: {:02x?}", processed_message);
-            CtapHid::split_message(processed_message)
-        } else {
-            HidPacketIterator::none()
-        }
-    }
-
     fn has_valid_channel(&self, message: &Message) -> bool {
         match message.cid {
             // Only INIT commands use the broadcast channel.
@@ -456,7 +379,7 @@ impl CtapHid {
     /// example long user names that are part of the output of an assertion. These cases should be
     /// correctly handled by the CTAP implementation. It is therefore an internal error from the
     /// HID perspective.
-    fn split_message(message: Message) -> HidPacketIterator {
+    pub fn split_message(message: Message) -> HidPacketIterator {
         let cid = message.cid;
         HidPacketIterator::new(message).unwrap_or_else(|| {
             // The error payload is 1 <= 7609 bytes, so unwrap() is safe.
@@ -478,40 +401,12 @@ impl CtapHid {
         })
     }
 
-    /// Returns whether a wink permission is currently granted.
-    pub fn should_wink(&self, now: CtapInstant) -> bool {
-        self.wink_permission.is_granted(now)
-    }
-
-    #[cfg(feature = "with_ctap1")]
-    fn ctap1_error_message(cid: ChannelID, error_code: ctap1::Ctap1StatusCode) -> Message {
-        let code: u16 = error_code.into();
-        Message {
-            cid,
-            cmd: CtapHidCommand::Msg,
-            payload: code.to_be_bytes().to_vec(),
-        }
-    }
-
-    #[cfg(feature = "with_ctap1")]
-    fn ctap1_success_message(cid: ChannelID, payload: &[u8]) -> Message {
-        let mut response = payload.to_vec();
-        let code: u16 = ctap1::Ctap1StatusCode::SW_SUCCESS.into();
-        response.extend_from_slice(&code.to_be_bytes());
-        Message {
-            cid,
-            cmd: CtapHidCommand::Msg,
-            payload: response,
-        }
-    }
-
     #[cfg(test)]
     pub fn new_initialized() -> (CtapHid, ChannelID) {
         (
             CtapHid {
                 assembler: MessageAssembler::new(),
                 allocated_cids: 1,
-                wink_permission: TimedPermission::waiting(),
             },
             [0x00, 0x00, 0x00, 0x01],
         )
@@ -674,67 +569,6 @@ mod test {
         assert_eq!(response, None);
     }
 
-    #[test]
-    fn test_process_hid_packet() {
-        let mut env = TestEnv::new();
-        let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
-        let (mut ctap_hid, cid) = CtapHid::new_initialized();
-
-        let mut ping_packet = [0x00; 64];
-        ping_packet[..4].copy_from_slice(&cid);
-        ping_packet[4..9].copy_from_slice(&[0x81, 0x00, 0x02, 0x99, 0x99]);
-
-        let mut response = ctap_hid.process_hid_packet(
-            &mut env,
-            &ping_packet,
-            CtapInstant::new(0),
-            &mut ctap_state,
-        );
-        assert_eq!(response.next(), Some(ping_packet));
-        assert_eq!(response.next(), None);
-    }
-
-    #[test]
-    fn test_process_hid_packet_empty() {
-        let mut env = TestEnv::new();
-        let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
-        let (mut ctap_hid, cid) = CtapHid::new_initialized();
-
-        let mut cancel_packet = [0x00; 64];
-        cancel_packet[..4].copy_from_slice(&cid);
-        cancel_packet[4..7].copy_from_slice(&[0x91, 0x00, 0x00]);
-
-        let mut response = ctap_hid.process_hid_packet(
-            &mut env,
-            &cancel_packet,
-            CtapInstant::new(0),
-            &mut ctap_state,
-        );
-        assert_eq!(response.next(), None);
-    }
-
-    #[test]
-    fn test_wink() {
-        let mut env = TestEnv::new();
-        let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
-        let (mut ctap_hid, cid) = CtapHid::new_initialized();
-        assert!(!ctap_hid.should_wink(CtapInstant::new(0)));
-
-        let mut wink_packet = [0x00; 64];
-        wink_packet[..4].copy_from_slice(&cid);
-        wink_packet[4..7].copy_from_slice(&[0x88, 0x00, 0x00]);
-
-        let mut response = ctap_hid.process_hid_packet(
-            &mut env,
-            &wink_packet,
-            CtapInstant::new(0),
-            &mut ctap_state,
-        );
-        assert_eq!(response.next(), Some(wink_packet));
-        assert_eq!(response.next(), None);
-        assert!(ctap_hid.should_wink(CtapInstant::new(0)));
-    }
-
     #[test]
     fn test_split_message() {
         let message = Message {

src/ctap/hid/receive.rs

@@ -1,4 +1,4 @@
-// Copyright 2019-2021 Google LLC
+// Copyright 2019-2022 Google LLC
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -21,7 +21,7 @@ use super::{
 use alloc::vec::Vec;
 use core::mem::swap;
 
-// A structure to assemble CTAPHID commands from a series of incoming USB HID packets.
+/// A structure to assemble CTAPHID commands from a series of incoming USB HID packets.
 pub struct MessageAssembler {
     // Whether this is waiting to receive an initialization packet.
     idle: bool,

src/ctap/hid/send.rs

@@ -1,4 +1,4 @@
-// Copyright 2019-2021 Google LLC
+// Copyright 2019-2022 Google LLC
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -53,8 +53,9 @@ struct MessageSplitter {
 }
 
 impl MessageSplitter {
-    // Try to split this message into an iterator of HID packets. This fails if the message is too
+    /// Try to split this message into an iterator of HID packets.
-    // long to fit into a sequence of HID packets (which is limited to 7609 bytes).
+    ///
+    /// This fails if the message is too long to fit into a sequence of HID packets.
     pub fn new(message: Message) -> Option<MessageSplitter> {
         if message.payload.len() > 7609 {
             None
@@ -72,9 +73,10 @@ impl MessageSplitter {
         }
     }
 
-    // Copy as many bytes as possible from data to dst, and return how many bytes are copied.
+    /// Copy as many bytes as possible from data to dst, and return how many bytes are copied.
-    // Contrary to copy_from_slice, this doesn't require slices of the same length.
+    ///
-    // All unused bytes in dst are set to zero, as if the data was padded with zeros to match.
+    /// Contrary to copy_from_slice, this doesn't require slices of the same length.
+    /// All unused bytes in dst are set to zero, as if the data was padded with zeros to match.
     fn consume_data(dst: &mut [u8], data: &[u8]) -> usize {
         let dst_len = dst.len();
         let data_len = data.len();

src/ctap/main_hid.rs

@@ -0,0 +1,229 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use super::CtapState;
+use crate::clock::{ClockInt, CtapInstant};
+#[cfg(feature = "with_ctap1")]
+use crate::ctap::ctap1;
+#[cfg(feature = "with_ctap1")]
+use crate::ctap::hid::ChannelID;
+use crate::ctap::hid::{
+    CtapHid, CtapHidCommand, CtapHidError, HidPacket, HidPacketIterator, Message,
+};
+use crate::ctap::TimedPermission;
+use crate::env::Env;
+use embedded_time::duration::Milliseconds;
+
+/// Implements the standard CTAP command processing for HID.
+pub struct MainHid {
+    hid: CtapHid,
+    wink_permission: TimedPermission,
+}
+
+impl MainHid {
+    const WINK_TIMEOUT_DURATION: Milliseconds<ClockInt> = Milliseconds(5000 as ClockInt);
+
+    /// Instantiates a HID handler for CTAP1, CTAP2 and Wink.
+    pub fn new() -> Self {
+        let hid = CtapHid::new();
+        let wink_permission = TimedPermission::waiting();
+        MainHid {
+            hid,
+            wink_permission,
+        }
+    }
+
+    /// Processes an incoming USB HID packet, and returns an iterator for all outgoing packets.
+    pub fn process_hid_packet(
+        &mut self,
+        env: &mut impl Env,
+        packet: &HidPacket,
+        now: CtapInstant,
+        ctap_state: &mut CtapState,
+    ) -> HidPacketIterator {
+        if let Some(message) = self.hid.parse_packet(env, packet, now) {
+            let processed_message = self.process_message(env, message, now, ctap_state);
+            debug_ctap!(env, "Sending message: {:02x?}", processed_message);
+            CtapHid::split_message(processed_message)
+        } else {
+            HidPacketIterator::none()
+        }
+    }
+
+    /// Processes a message's commands that affect the protocol outside HID.
+    pub fn process_message(
+        &mut self,
+        env: &mut impl Env,
+        message: Message,
+        now: CtapInstant,
+        ctap_state: &mut CtapState,
+    ) -> Message {
+        // If another command arrives, stop winking to prevent accidential button touches.
+        self.wink_permission = TimedPermission::waiting();
+
+        let cid = message.cid;
+        match message.cmd {
+            // CTAP 2.1 from 2021-06-15, section 11.2.9.1.1.
+            CtapHidCommand::Msg => {
+                // If we don't have CTAP1 backward compatibilty, this command is invalid.
+                #[cfg(not(feature = "with_ctap1"))]
+                return CtapHid::error_message(cid, CtapHidError::InvalidCmd);
+
+                #[cfg(feature = "with_ctap1")]
+                match ctap1::Ctap1Command::process_command(env, &message.payload, ctap_state, now) {
+                    Ok(payload) => MainHid::ctap1_success_message(cid, &payload),
+                    Err(ctap1_status_code) => MainHid::ctap1_error_message(cid, ctap1_status_code),
+                }
+            }
+            // CTAP 2.1 from 2021-06-15, section 11.2.9.1.2.
+            CtapHidCommand::Cbor => {
+                // Each transaction is atomic, so we process the command directly here and
+                // don't handle any other packet in the meantime.
+                // TODO: Send "Processing" type keep-alive packets in the meantime.
+                let response = ctap_state.process_command(env, &message.payload, cid, now);
+                Message {
+                    cid,
+                    cmd: CtapHidCommand::Cbor,
+                    payload: response,
+                }
+            }
+            // CTAP 2.1 from 2021-06-15, section 11.2.9.2.1.
+            CtapHidCommand::Wink => {
+                if message.payload.is_empty() {
+                    self.wink_permission =
+                        TimedPermission::granted(now, Self::WINK_TIMEOUT_DURATION);
+                    // The response is empty like the request.
+                    message
+                } else {
+                    CtapHid::error_message(cid, CtapHidError::InvalidLen)
+                }
+            }
+            // All other commands have already been processed, keep them as is.
+            _ => message,
+        }
+    }
+
+    /// Returns whether a wink permission is currently granted.
+    pub fn should_wink(&self, now: CtapInstant) -> bool {
+        self.wink_permission.is_granted(now)
+    }
+
+    /// Updates the timeout for the wink permission.
+    pub fn update_wink_timeout(&mut self, now: CtapInstant) {
+        self.wink_permission = self.wink_permission.check_expiration(now);
+    }
+
+    #[cfg(feature = "with_ctap1")]
+    fn ctap1_error_message(cid: ChannelID, error_code: ctap1::Ctap1StatusCode) -> Message {
+        let code: u16 = error_code.into();
+        Message {
+            cid,
+            cmd: CtapHidCommand::Msg,
+            payload: code.to_be_bytes().to_vec(),
+        }
+    }
+
+    #[cfg(feature = "with_ctap1")]
+    fn ctap1_success_message(cid: ChannelID, payload: &[u8]) -> Message {
+        let mut response = payload.to_vec();
+        let code: u16 = ctap1::Ctap1StatusCode::SW_SUCCESS.into();
+        response.extend_from_slice(&code.to_be_bytes());
+        Message {
+            cid,
+            cmd: CtapHidCommand::Msg,
+            payload: response,
+        }
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::ctap::hid::ChannelID;
+    use crate::env::test::TestEnv;
+
+    fn new_initialized() -> (MainHid, ChannelID) {
+        let (hid, cid) = CtapHid::new_initialized();
+        let wink_permission = TimedPermission::waiting();
+        (
+            MainHid {
+                hid,
+                wink_permission,
+            },
+            cid,
+        )
+    }
+
+    #[test]
+    fn test_process_hid_packet() {
+        let mut env = TestEnv::new();
+        let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
+        let (mut main_hid, cid) = new_initialized();
+
+        let mut ping_packet = [0x00; 64];
+        ping_packet[..4].copy_from_slice(&cid);
+        ping_packet[4..9].copy_from_slice(&[0x81, 0x00, 0x02, 0x99, 0x99]);
+
+        let mut response = main_hid.process_hid_packet(
+            &mut env,
+            &ping_packet,
+            CtapInstant::new(0),
+            &mut ctap_state,
+        );
+        assert_eq!(response.next(), Some(ping_packet));
+        assert_eq!(response.next(), None);
+    }
+
+    #[test]
+    fn test_process_hid_packet_empty() {
+        let mut env = TestEnv::new();
+        let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
+        let (mut main_hid, cid) = new_initialized();
+
+        let mut cancel_packet = [0x00; 64];
+        cancel_packet[..4].copy_from_slice(&cid);
+        cancel_packet[4..7].copy_from_slice(&[0x91, 0x00, 0x00]);
+
+        let mut response = main_hid.process_hid_packet(
+            &mut env,
+            &cancel_packet,
+            CtapInstant::new(0),
+            &mut ctap_state,
+        );
+        assert_eq!(response.next(), None);
+    }
+
+    #[test]
+    fn test_wink() {
+        let mut env = TestEnv::new();
+        let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
+        let (mut main_hid, cid) = new_initialized();
+        assert!(!main_hid.should_wink(CtapInstant::new(0)));
+
+        let mut wink_packet = [0x00; 64];
+        wink_packet[..4].copy_from_slice(&cid);
+        wink_packet[4..7].copy_from_slice(&[0x88, 0x00, 0x00]);
+
+        let mut response = main_hid.process_hid_packet(
+            &mut env,
+            &wink_packet,
+            CtapInstant::new(0),
+            &mut ctap_state,
+        );
+        assert_eq!(response.next(), Some(wink_packet));
+        assert_eq!(response.next(), None);
+        assert!(main_hid.should_wink(CtapInstant::new(0)));
+        assert!(!main_hid.should_wink(CtapInstant::new(1) + MainHid::WINK_TIMEOUT_DURATION));
+    }
+}

src/ctap/mod.rs

@@ -25,6 +25,7 @@ pub mod data_formats;
 pub mod hid;
 mod key_material;
 mod large_blobs;
+pub mod main_hid;
 mod pin_protocol;
 pub mod response;
 pub mod status_code;

src/lib.rs

@@ -1,4 +1,4 @@
-// Copyright 2019 Google LLC
+// Copyright 2019-2022 Google LLC
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -18,7 +18,8 @@ extern crate alloc;
 #[macro_use]
 extern crate arrayref;
 
-use crate::ctap::hid::{CtapHid, HidPacket, HidPacketIterator};
+use crate::ctap::hid::{HidPacket, HidPacketIterator};
+use crate::ctap::main_hid::MainHid;
 use crate::ctap::CtapState;
 use crate::env::Env;
 use clock::CtapInstant;
@@ -53,7 +54,7 @@ pub mod env;
 pub struct Ctap<E: Env> {
     env: E,
     state: CtapState,
-    hid: CtapHid,
+    hid: MainHid,
 }
 
 impl<E: Env> Ctap<E> {
@@ -62,7 +63,7 @@ impl<E: Env> Ctap<E> {
     // clock is part of the environment.
     pub fn new(mut env: E, now: CtapInstant) -> Self {
         let state = CtapState::new(&mut env, now);
-        let hid = CtapHid::new();
+        let hid = MainHid::new();
         Ctap { env, state, hid }
     }
 
@@ -70,7 +71,7 @@ impl<E: Env> Ctap<E> {
         &mut self.state
     }
 
-    pub fn hid(&mut self) -> &mut CtapHid {
+    pub fn hid(&mut self) -> &mut MainHid {
         &mut self.hid
     }
 
@@ -85,6 +86,6 @@ impl<E: Env> Ctap<E> {
 
     pub fn update_timeouts(&mut self, now: CtapInstant) {
         self.state.update_timeouts(now);
-        self.hid.wink_permission = self.hid.wink_permission.check_expiration(now);
+        self.hid.update_wink_timeout(now);
     }
 }