From 0b564d4a8a848af7d4f032950f4ce886da9b71ad Mon Sep 17 00:00:00 2001 From: kaczmarczyck <43844792+kaczmarczyck@users.noreply.github.com> Date: Tue, 15 Mar 2022 14:41:48 +0100 Subject: [PATCH] Vendor HID (#446) * introduces vendor HID * updates workflows with new feature * feature renaming and variant covering --- .github/workflows/cargo_check.yml | 10 +- .github/workflows/coveralls.yml | 2 +- .github/workflows/opensk_build.yml | 2 +- Cargo.toml | 1 + deploy.py | 7 ++ run_desktop_tests.sh | 10 +- src/ctap/main_hid.rs | 3 +- src/ctap/mod.rs | 4 + src/ctap/vendor_hid.rs | 174 +++++++++++++++++++++++++++++ src/env/tock/mod.rs | 1 + src/lib.rs | 20 +++- 11 files changed, 222 insertions(+), 12 deletions(-) create mode 100644 src/ctap/vendor_hid.rs diff --git a/.github/workflows/cargo_check.yml b/.github/workflows/cargo_check.yml index 185b028..5c56ff7 100644 --- a/.github/workflows/cargo_check.yml +++ b/.github/workflows/cargo_check.yml @@ -41,6 +41,12 @@ jobs: command: check args: --target thumbv7em-none-eabi --release --features with_ctap1 + - name: Check OpenSK vendor_hid + uses: actions-rs/cargo@v1 + with: + command: check + args: --target thumbv7em-none-eabi --release --features vendor_hid + - name: Check OpenSK debug_ctap uses: actions-rs/cargo@v1 with: @@ -71,11 +77,11 @@ jobs: command: check args: --target thumbv7em-none-eabi --release --features debug_ctap,with_ctap1 - - name: Check OpenSK debug_ctap,with_ctap1,panic_console,debug_allocations,verbose + - name: Check OpenSK debug_ctap,with_ctap1,vendor_hid,panic_console,debug_allocations,verbose uses: actions-rs/cargo@v1 with: command: check - args: --target thumbv7em-none-eabi --release --features debug_ctap,with_ctap1,,panic_console,debug_allocations,verbose + args: --target thumbv7em-none-eabi --release --features debug_ctap,with_ctap1,vendor_hid,panic_console,debug_allocations,verbose - name: Check examples uses: actions-rs/cargo@v1 diff --git a/.github/workflows/coveralls.yml b/.github/workflows/coveralls.yml index 474829c..e405ab6 100644 --- a/.github/workflows/coveralls.yml +++ b/.github/workflows/coveralls.yml @@ -34,7 +34,7 @@ jobs: - uses: actions-rs/cargo@v1 with: command: test - args: --features "with_ctap1,with_nfc,std" --no-fail-fast + args: --features "with_ctap1,vendor_hid,with_nfc,std" --no-fail-fast env: RUSTFLAGS: "-Zinstrument-coverage" LLVM_PROFILE_FILE: "opensk-%p-%m.profraw" diff --git a/.github/workflows/opensk_build.yml b/.github/workflows/opensk_build.yml index f7e73ae..ea1f65a 100644 --- a/.github/workflows/opensk_build.yml +++ b/.github/workflows/opensk_build.yml @@ -35,6 +35,6 @@ jobs: uses: actions-rs/cargo@v1 with: command: build - args: --release --target=thumbv7em-none-eabi --features with_ctap1 + args: --release --target=thumbv7em-none-eabi --features with_ctap1,vendor_hid - name: Compute SHA-256 sum run: ./third_party/tock/tools/sha256sum/target/debug/sha256sum target/thumbv7em-none-eabi/release/ctap2 diff --git a/Cargo.toml b/Cargo.toml index 84f0f8f..c55b09a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ std = ["crypto/std", "lang_items/std", "persistent_store/std"] verbose = ["debug_ctap", "libtock_drivers/verbose_usb"] with_ctap1 = ["crypto/with_ctap1"] with_nfc = ["libtock_drivers/with_nfc"] +vendor_hid = [] [dev-dependencies] enum-iterator = "0.6.0" diff --git a/deploy.py b/deploy.py index cd22078..4161bf1 100755 --- a/deploy.py +++ b/deploy.py @@ -1057,6 +1057,13 @@ if __name__ == "__main__": dest="features", help=("Compiles the OpenSK application with support for nfc."), ) + main_parser.add_argument( + "--vendor-hid", + action="append_const", + const="vendor_hid", + dest="features", + help=("Compiles the OpenSK application to support two HID usage pages."), + ) main_parser.add_argument( "--regen-keys", action="store_true", diff --git a/run_desktop_tests.sh b/run_desktop_tests.sh index 3c0e9f4..276db76 100755 --- a/run_desktop_tests.sh +++ b/run_desktop_tests.sh @@ -36,6 +36,7 @@ cd .. echo "Running Clippy lints..." cargo clippy --all-targets --features std -- -A clippy::new_without_default -D warnings cargo clippy --all-targets --features std,with_nfc -- -A clippy::new_without_default -D warnings +cargo clippy --all-targets --features std,vendor_hid -- -A clippy::new_without_default -D warnings echo "Building sha256sum tool..." cargo build --manifest-path third_party/tock/tools/sha256sum/Cargo.toml @@ -47,12 +48,13 @@ cargo test --manifest-path tools/heapviz/Cargo.toml echo "Checking that CTAP2 builds properly..." cargo check --release --target=thumbv7em-none-eabi cargo check --release --target=thumbv7em-none-eabi --features with_ctap1 +cargo check --release --target=thumbv7em-none-eabi --features vendor_hid cargo check --release --target=thumbv7em-none-eabi --features debug_ctap cargo check --release --target=thumbv7em-none-eabi --features panic_console cargo check --release --target=thumbv7em-none-eabi --features debug_allocations cargo check --release --target=thumbv7em-none-eabi --features verbose cargo check --release --target=thumbv7em-none-eabi --features debug_ctap,with_ctap1 -cargo check --release --target=thumbv7em-none-eabi --features debug_ctap,with_ctap1,panic_console,debug_allocations,verbose +cargo check --release --target=thumbv7em-none-eabi --features debug_ctap,with_ctap1,vendor_hid,panic_console,debug_allocations,verbose echo "Checking that examples build properly..." cargo check --release --target=thumbv7em-none-eabi --examples @@ -118,9 +120,9 @@ then cd ../.. cargo test --features std - echo "Running unit tests on the desktop (release mode + CTAP1)..." - cargo test --release --features std,with_ctap1 + echo "Running unit tests on the desktop (release mode + CTAP1 + Vendor HID)..." + cargo test --release --features std,with_ctap1,vendor_hid echo "Running unit tests on the desktop (debug mode + CTAP1)..." - cargo test --features std,with_ctap1 + cargo test --features std,with_ctap1,vendor_hid fi diff --git a/src/ctap/main_hid.rs b/src/ctap/main_hid.rs index ae11587..1132176 100644 --- a/src/ctap/main_hid.rs +++ b/src/ctap/main_hid.rs @@ -12,7 +12,6 @@ // 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; @@ -21,7 +20,7 @@ use crate::ctap::hid::ChannelID; use crate::ctap::hid::{ CtapHid, CtapHidCommand, CtapHidError, HidPacket, HidPacketIterator, Message, }; -use crate::ctap::{Channel, TimedPermission}; +use crate::ctap::{Channel, CtapState, TimedPermission}; use crate::env::Env; use embedded_time::duration::Milliseconds; diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 870a9df..66afb90 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -32,6 +32,8 @@ pub mod status_code; mod storage; mod timed_permission; mod token_state; +#[cfg(feature = "vendor_hid")] +pub mod vendor_hid; use self::client_pin::{ClientPin, PinPermission}; use self::command::{ @@ -136,6 +138,7 @@ pub enum Transport { /// Corresponds to CTAP's USB transport. MainHid, /// No equivalent in CTAP, used for communication outside the specification. + #[cfg(feature = "vendor_hid")] VendorHid, } @@ -151,6 +154,7 @@ pub enum Channel { /// Corresponds to CTAP's USB transport. MainHid(ChannelID), /// No equivalent in CTAP, used for communication outside the specification. + #[cfg(feature = "vendor_hid")] VendorHid(ChannelID), } diff --git a/src/ctap/vendor_hid.rs b/src/ctap/vendor_hid.rs new file mode 100644 index 0000000..83bfc0f --- /dev/null +++ b/src/ctap/vendor_hid.rs @@ -0,0 +1,174 @@ +// 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 crate::clock::CtapInstant; +use crate::ctap::hid::{ + CtapHid, CtapHidCommand, CtapHidError, HidPacket, HidPacketIterator, Message, +}; +use crate::ctap::{Channel, CtapState}; +use crate::env::Env; + +/// Implements the non-standard command processing for HID. +/// +/// Outside of the pure HID commands like INIT, only PING and CBOR commands are allowed. +pub struct VendorHid { + hid: CtapHid, +} + +impl VendorHid { + /// Instantiates a HID handler for CTAP1, CTAP2 and Wink. + pub fn new() -> Self { + let hid = CtapHid::new(); + VendorHid { hid } + } + + /// 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 through the second usage page: {: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 { + let cid = message.cid; + match message.cmd { + // There are no custom CTAP1 commands. + CtapHidCommand::Msg => CtapHid::error_message(cid, CtapHidError::InvalidCmd), + // The CTAP2 processing function multiplexes internally. + CtapHidCommand::Cbor => { + let response = + ctap_state.process_command(env, &message.payload, Channel::VendorHid(cid), now); + Message { + cid, + cmd: CtapHidCommand::Cbor, + payload: response, + } + } + // Call Wink over the main HID. + CtapHidCommand::Wink => CtapHid::error_message(cid, CtapHidError::InvalidCmd), + // All other commands have already been processed, keep them as is. + _ => message, + } + } +} + +#[cfg(test)] +mod test { + use super::*; + use crate::ctap::hid::ChannelID; + use crate::env::test::TestEnv; + + fn new_initialized() -> (VendorHid, ChannelID) { + let (hid, cid) = CtapHid::new_initialized(); + (VendorHid { hid }, 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 vendor_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 = vendor_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 vendor_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 = vendor_hid.process_hid_packet( + &mut env, + &cancel_packet, + CtapInstant::new(0), + &mut ctap_state, + ); + assert_eq!(response.next(), None); + } + + #[test] + fn test_blocked_commands() { + let mut env = TestEnv::new(); + let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); + let (mut vendor_hid, cid) = new_initialized(); + + // Usually longer, but we don't parse them anyway. + let mut msg_packet = [0x00; 64]; + msg_packet[..4].copy_from_slice(&cid); + msg_packet[4..7].copy_from_slice(&[0x83, 0x00, 0x00]); + + 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 error_packet = [0x00; 64]; + error_packet[..4].copy_from_slice(&cid); + error_packet[4..8].copy_from_slice(&[0xBF, 0x00, 0x01, 0x01]); + + let mut response = vendor_hid.process_hid_packet( + &mut env, + &msg_packet, + CtapInstant::new(0), + &mut ctap_state, + ); + assert_eq!(response.next(), Some(error_packet)); + assert_eq!(response.next(), None); + + let mut response = vendor_hid.process_hid_packet( + &mut env, + &wink_packet, + CtapInstant::new(0), + &mut ctap_state, + ); + assert_eq!(response.next(), Some(error_packet)); + assert_eq!(response.next(), None); + } +} diff --git a/src/env/tock/mod.rs b/src/env/tock/mod.rs index b7de1b0..4801a95 100644 --- a/src/env/tock/mod.rs +++ b/src/env/tock/mod.rs @@ -58,6 +58,7 @@ impl UserPresence for TockEnv { fn check(&mut self, channel: Channel) -> Result<(), Ctap2StatusCode> { match channel { Channel::MainHid(cid) => check_user_presence(self, cid), + #[cfg(feature = "vendor_hid")] Channel::VendorHid(cid) => check_user_presence(self, cid), } } diff --git a/src/lib.rs b/src/lib.rs index 055c422..4e23b2e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,6 +20,8 @@ extern crate arrayref; use crate::ctap::hid::{HidPacket, HidPacketIterator}; use crate::ctap::main_hid::MainHid; +#[cfg(feature = "vendor_hid")] +use crate::ctap::vendor_hid::VendorHid; use crate::ctap::CtapState; pub use crate::ctap::Transport; use crate::env::Env; @@ -56,6 +58,8 @@ pub struct Ctap { env: E, state: CtapState, hid: MainHid, + #[cfg(feature = "vendor_hid")] + vendor_hid: VendorHid, } impl Ctap { @@ -65,7 +69,15 @@ impl Ctap { pub fn new(mut env: E, now: CtapInstant) -> Self { let state = CtapState::new(&mut env, now); let hid = MainHid::new(); - Ctap { env, state, hid } + #[cfg(feature = "vendor_hid")] + let vendor_hid = VendorHid::new(); + Ctap { + env, + state, + hid, + #[cfg(feature = "vendor_hid")] + vendor_hid, + } } pub fn state(&mut self) -> &mut CtapState { @@ -87,7 +99,11 @@ impl Ctap { self.hid .process_hid_packet(&mut self.env, packet, now, &mut self.state) } - Transport::VendorHid => HidPacketIterator::none(), + #[cfg(feature = "vendor_hid")] + Transport::VendorHid => { + self.vendor_hid + .process_hid_packet(&mut self.env, packet, now, &mut self.state) + } } }