From 1e9579d55d71d35d14ca997d0d5fdcde295c538f Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Tue, 9 Jan 2024 15:30:24 +0100 Subject: [PATCH 1/2] Allows initialization without Reset permission This PR is useful for all implementations that can trigger a reboot without user intervention. In these cases, we don't want to allow the Reset command. It should only be allowed after a user initiated power cycle. Adds tests to the new functionality and a few other coverage holes. --- libraries/opensk/fuzz/fuzz_helper/src/lib.rs | 4 +- libraries/opensk/src/api/attestation_store.rs | 11 +++ libraries/opensk/src/api/connection.rs | 16 +++- libraries/opensk/src/api/customization.rs | 43 ++++++++++ libraries/opensk/src/ctap/mod.rs | 11 ++- libraries/opensk/src/lib.rs | 78 ++++++++++++++++--- src/main.rs | 2 +- 7 files changed, 151 insertions(+), 14 deletions(-) diff --git a/libraries/opensk/fuzz/fuzz_helper/src/lib.rs b/libraries/opensk/fuzz/fuzz_helper/src/lib.rs index cab68a59..4fcb5c57 100644 --- a/libraries/opensk/fuzz/fuzz_helper/src/lib.rs +++ b/libraries/opensk/fuzz/fuzz_helper/src/lib.rs @@ -141,7 +141,7 @@ pub fn process_ctap_any_type(data: &[u8]) -> arbitrary::Result<()> { let data = unstructured.take_rest(); // Initialize ctap state and hid and get the allocated cid. - let mut ctap = Ctap::new(env); + let mut ctap = Ctap::new(env, false); let cid = initialize(&mut ctap); // Wrap input as message with the allocated cid. let mut command = cid.to_vec(); @@ -191,7 +191,7 @@ pub fn process_ctap_specific_type(data: &[u8], input_type: InputType) -> arbitra return Ok(()); } // Initialize ctap state and hid and get the allocated cid. - let mut ctap = Ctap::new(env); + let mut ctap = Ctap::new(env, false); let cid = initialize(&mut ctap); // Wrap input as message with allocated cid and command type. let mut command = cid.to_vec(); diff --git a/libraries/opensk/src/api/attestation_store.rs b/libraries/opensk/src/api/attestation_store.rs index f4cfb894..68dcee27 100644 --- a/libraries/opensk/src/api/attestation_store.rs +++ b/libraries/opensk/src/api/attestation_store.rs @@ -113,3 +113,14 @@ impl From for Error { } } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_store_error() { + assert_eq!(Error::from(StoreError::StorageError), Error::Storage); + assert_eq!(Error::from(StoreError::InvalidStorage), Error::Internal); + } +} diff --git a/libraries/opensk/src/api/connection.rs b/libraries/opensk/src/api/connection.rs index 348c5f4d..b3a577ce 100644 --- a/libraries/opensk/src/api/connection.rs +++ b/libraries/opensk/src/api/connection.rs @@ -14,7 +14,7 @@ use core::convert::TryFrom; -#[derive(Clone, Copy, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum UsbEndpoint { MainHid = 1, #[cfg(feature = "vendor_hid")] @@ -40,6 +40,7 @@ pub enum SendOrRecvStatus { Received(UsbEndpoint), } +#[derive(Debug, PartialEq, Eq)] pub struct SendOrRecvError; pub type SendOrRecvResult = Result; @@ -47,3 +48,16 @@ pub type SendOrRecvResult = Result; pub trait HidConnection { fn send_and_maybe_recv(&mut self, buf: &mut [u8; 64], timeout_ms: usize) -> SendOrRecvResult; } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_endpoint_num() { + assert_eq!(UsbEndpoint::try_from(1), Ok(UsbEndpoint::MainHid)); + #[cfg(feature = "vendor_hid")] + assert_eq!(UsbEndpoint::try_from(2), Ok(UsbEndpoint::VendorHid)); + assert_eq!(UsbEndpoint::try_from(3), Err(SendOrRecvError)); + } +} diff --git a/libraries/opensk/src/api/customization.rs b/libraries/opensk/src/api/customization.rs index 98b367fc..38c6a426 100644 --- a/libraries/opensk/src/api/customization.rs +++ b/libraries/opensk/src/api/customization.rs @@ -457,4 +457,47 @@ mod test { fn test_invariants() { assert!(is_valid(&DEFAULT_CUSTOMIZATION)); } + + #[test] + fn test_accessors() { + let customization = CustomizationImpl { + aaguid: &[0; AAGUID_LENGTH], + allows_pin_protocol_v1: true, + default_cred_protect: None, + default_min_pin_length: 4, + default_min_pin_length_rp_ids: &["example.com"], + enforce_always_uv: false, + enterprise_attestation_mode: None, + enterprise_rp_id_list: &[], + max_msg_size: 7609, + max_pin_retries: 8, + use_batch_attestation: true, + use_signature_counter: true, + max_cred_blob_length: 32, + max_credential_count_in_list: Some(3), + max_large_blob_array_size: 2048, + max_rp_ids_length: 8, + max_supported_resident_keys: 150, + }; + assert_eq!(customization.aaguid(), &[0; AAGUID_LENGTH]); + assert!(customization.allows_pin_protocol_v1()); + assert!(customization.default_cred_protect().is_none()); + assert_eq!(customization.default_min_pin_length(), 4); + assert_eq!( + customization.default_min_pin_length_rp_ids(), + vec![String::from("example.com")] + ); + assert!(!customization.enforce_always_uv()); + assert!(customization.enterprise_attestation_mode().is_none()); + assert!(customization.enterprise_rp_id_list().is_empty()); + assert_eq!(customization.max_msg_size(), 7609); + assert_eq!(customization.max_pin_retries(), 8); + assert!(customization.use_batch_attestation()); + assert!(customization.use_signature_counter()); + assert_eq!(customization.max_cred_blob_length(), 32); + assert_eq!(customization.max_credential_count_in_list(), Some(3)); + assert_eq!(customization.max_large_blob_array_size(), 2048); + assert_eq!(customization.max_rp_ids_length(), 8); + assert_eq!(customization.max_supported_resident_keys(), 150); + } } diff --git a/libraries/opensk/src/ctap/mod.rs b/libraries/opensk/src/ctap/mod.rs index 748f2566..d3142dda 100644 --- a/libraries/opensk/src/ctap/mod.rs +++ b/libraries/opensk/src/ctap/mod.rs @@ -413,7 +413,9 @@ pub struct StatefulPermission { impl StatefulPermission { /// Creates the command state at device startup. /// - /// Resets are only possible after a power cycle. Therefore, initialization + /// Resets are only possible after a power cycle. Therefore, there is no way to grant the Reset + /// permission outside of this function. If you initialize the app without a power cycle + /// (potentially after waking up from sleep), `clear` the permissions. /// means allowing Reset, and Reset cannot be granted later. pub fn new_reset(env: &mut E) -> StatefulPermission { StatefulPermission { @@ -552,6 +554,13 @@ impl CtapState { } } + /// Creates new CTAP state that doesn't assume a user intended power cycle. + pub fn new_soft_reset(env: &mut E) -> Self { + let mut ctap_state = CtapState::new(env); + ctap_state.stateful_command_permission.clear(); + ctap_state + } + pub fn increment_global_signature_counter( &mut self, env: &mut E, diff --git a/libraries/opensk/src/lib.rs b/libraries/opensk/src/lib.rs index 1412a95a..adbf5ef9 100644 --- a/libraries/opensk/src/lib.rs +++ b/libraries/opensk/src/lib.rs @@ -63,8 +63,12 @@ impl Ctap { /// Instantiates a CTAP implementation given its environment. // This should only take the environment, but it temporarily takes the boot time until the // clock is part of the environment. - pub fn new(mut env: E) -> Self { - let state = CtapState::::new(&mut env); + pub fn new(mut env: E, is_soft_reset: bool) -> Self { + let state = if is_soft_reset { + CtapState::::new_soft_reset(&mut env) + } else { + CtapState::::new(&mut env) + }; let hid = MainHid::default(); #[cfg(feature = "vendor_hid")] let vendor_hid = VendorHid::default(); @@ -81,10 +85,6 @@ impl Ctap { &mut self.state } - pub fn hid(&mut self) -> &mut MainHid { - &mut self.hid - } - pub fn env(&mut self) -> &mut E { &mut self.env } @@ -134,6 +134,7 @@ impl Ctap { #[cfg(test)] mod test { use super::*; + use crate::ctap::status_code::Ctap2StatusCode; use crate::env::test::TestEnv; /// Assembles a packet for a payload that fits into one packet. @@ -162,7 +163,7 @@ mod test { #[test] fn test_wink() { let env = TestEnv::default(); - let mut ctap = Ctap::::new(env); + let mut ctap = Ctap::::new(env, false); // Send Init, receive Init response and check wink if disabled. let mut init_response = ctap.process_hid_packet(&init_packet(), Transport::MainHid); @@ -181,7 +182,7 @@ mod test { #[test] fn test_locked_channel_id() { let env = TestEnv::default(); - let mut ctap = Ctap::::new(env); + let mut ctap = Ctap::::new(env, false); // Send Init, receive Init response. let mut init_response = ctap.process_hid_packet(&init_packet(), Transport::MainHid); @@ -200,11 +201,60 @@ mod test { assert_eq!(response_packet[4], 0xBF); } + #[test] + fn test_hard_reset() { + let env = TestEnv::default(); + let mut ctap = Ctap::::new(env, false); + + // Send Init, receive Init response. + let mut init_response = ctap.process_hid_packet(&init_packet(), Transport::MainHid); + let response_packet = init_response.next().unwrap(); + assert_eq!(response_packet[4], 0x86); + let cid = *array_ref!(response_packet, 15, 4); + + // Send Reset, get Ok. + let reset_packet = assemble_packet(&cid, 0x10, &[0x07]); + let mut reset_response = ctap.process_hid_packet(&reset_packet, Transport::MainHid); + let response_packet = reset_response.next().unwrap(); + let status_byte = Ctap2StatusCode::CTAP2_OK as u8; + let expected_data = [0x90, 0x00, 0x01, status_byte]; + assert_eq!(response_packet[..4], cid); + assert_eq!(response_packet[4..8], expected_data); + } + + #[test] + fn test_soft_reset() { + let env = TestEnv::default(); + let mut ctap = Ctap::::new(env, true); + + // Send Init, receive Init response. + let mut init_response = ctap.process_hid_packet(&init_packet(), Transport::MainHid); + let response_packet = init_response.next().unwrap(); + assert_eq!(response_packet[4], 0x86); + let cid = *array_ref!(response_packet, 15, 4); + + // Send Reset, get error. + let reset_packet = assemble_packet(&cid, 0x10, &[0x07]); + let mut reset_response = ctap.process_hid_packet(&reset_packet, Transport::MainHid); + let response_packet = reset_response.next().unwrap(); + let status_byte = Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED as u8; + let expected_data = [0x90, 0x00, 0x01, status_byte]; + assert_eq!(response_packet[..4], cid); + assert_eq!(response_packet[4..8], expected_data); + } + + #[test] + fn test_env_api() { + let env = TestEnv::default(); + let mut ctap = Ctap::::new(env, true); + assert_eq!(ctap.env().firmware_version(), Some(0)); + } + #[test] #[cfg(feature = "vendor_hid")] fn test_locked_transport() { let env = TestEnv::default(); - let mut ctap = Ctap::::new(env); + let mut ctap = Ctap::::new(env, false); // Send Init, receive Init response. let mut init_response = ctap.process_hid_packet(&init_packet(), Transport::MainHid); @@ -222,4 +272,14 @@ mod test { let response_packet = init_response.next().unwrap(); assert_eq!(response_packet[4], 0xBF); } + + #[test] + #[cfg(feature = "with_ctap1")] + fn test_ctap1_initial_state() { + let env = TestEnv::default(); + let mut ctap = Ctap::::new(env, false); + // Granting doesn't work until a CTAP1 request was processed. + ctap.u2f_grant_user_presence(); + assert!(!ctap.u2f_needs_user_presence()); + } } diff --git a/src/main.rs b/src/main.rs index a71c8ff9..0676b553 100644 --- a/src/main.rs +++ b/src/main.rs @@ -137,7 +137,7 @@ fn main() { } let env = TockEnv::::default(); - let mut ctap = opensk::Ctap::new(env); + let mut ctap = opensk::Ctap::new(env, false); let mut led_counter = 0; let mut led_blink_timer = From 1bf86118552eb6811c35c52c30f661aba3265c99 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Tue, 9 Jan 2024 17:47:14 +0100 Subject: [PATCH 2/2] Moves soft reset parameters into Env --- libraries/opensk/fuzz/fuzz_helper/src/lib.rs | 4 +-- libraries/opensk/src/ctap/mod.rs | 31 +++++++++++++------- libraries/opensk/src/env/mod.rs | 3 ++ libraries/opensk/src/env/test/mod.rs | 20 +++++++++++++ libraries/opensk/src/lib.rs | 25 +++++++--------- src/env/tock/mod.rs | 4 +++ src/main.rs | 2 +- 7 files changed, 62 insertions(+), 27 deletions(-) diff --git a/libraries/opensk/fuzz/fuzz_helper/src/lib.rs b/libraries/opensk/fuzz/fuzz_helper/src/lib.rs index 4fcb5c57..cab68a59 100644 --- a/libraries/opensk/fuzz/fuzz_helper/src/lib.rs +++ b/libraries/opensk/fuzz/fuzz_helper/src/lib.rs @@ -141,7 +141,7 @@ pub fn process_ctap_any_type(data: &[u8]) -> arbitrary::Result<()> { let data = unstructured.take_rest(); // Initialize ctap state and hid and get the allocated cid. - let mut ctap = Ctap::new(env, false); + let mut ctap = Ctap::new(env); let cid = initialize(&mut ctap); // Wrap input as message with the allocated cid. let mut command = cid.to_vec(); @@ -191,7 +191,7 @@ pub fn process_ctap_specific_type(data: &[u8], input_type: InputType) -> arbitra return Ok(()); } // Initialize ctap state and hid and get the allocated cid. - let mut ctap = Ctap::new(env, false); + let mut ctap = Ctap::new(env); let cid = initialize(&mut ctap); // Wrap input as message with allocated cid and command type. let mut command = cid.to_vec(); diff --git a/libraries/opensk/src/ctap/mod.rs b/libraries/opensk/src/ctap/mod.rs index d3142dda..509179d2 100644 --- a/libraries/opensk/src/ctap/mod.rs +++ b/libraries/opensk/src/ctap/mod.rs @@ -410,13 +410,26 @@ pub struct StatefulPermission { channel: Option, } +impl Default for StatefulPermission { + /// Creates the command state at device startup without user action. + /// + /// Reset is not granted after a forced reboot. The user replugging the device is a required + /// to avoid accidental data loss. + fn default() -> StatefulPermission { + StatefulPermission { + permission: ::Timer::default(), + command_type: None, + channel: None, + } + } +} + impl StatefulPermission { /// Creates the command state at device startup. /// /// Resets are only possible after a power cycle. Therefore, there is no way to grant the Reset /// permission outside of this function. If you initialize the app without a power cycle - /// (potentially after waking up from sleep), `clear` the permissions. - /// means allowing Reset, and Reset cannot be granted later. + /// (potentially after waking up from sleep), call `default` instead. pub fn new_reset(env: &mut E) -> StatefulPermission { StatefulPermission { permission: env.clock().make_timer(RESET_TIMEOUT_DURATION_MS), @@ -545,22 +558,20 @@ impl CtapState { pub fn new(env: &mut E) -> Self { storage::init(env).ok().unwrap(); let client_pin = ClientPin::new(env); + let stateful_command_permission = if env.boots_after_soft_reset() { + StatefulPermission::default() + } else { + StatefulPermission::new_reset(env) + }; CtapState { client_pin, #[cfg(feature = "with_ctap1")] u2f_up_state: U2fUserPresenceState::new(), - stateful_command_permission: StatefulPermission::new_reset(env), + stateful_command_permission, large_blobs: LargeBlobs::new(), } } - /// Creates new CTAP state that doesn't assume a user intended power cycle. - pub fn new_soft_reset(env: &mut E) -> Self { - let mut ctap_state = CtapState::new(env); - ctap_state.stateful_command_permission.clear(); - ctap_state - } - pub fn increment_global_signature_counter( &mut self, env: &mut E, diff --git a/libraries/opensk/src/env/mod.rs b/libraries/opensk/src/env/mod.rs index 01800bed..7130a034 100644 --- a/libraries/opensk/src/env/mod.rs +++ b/libraries/opensk/src/env/mod.rs @@ -76,6 +76,9 @@ pub trait Env { #[cfg(feature = "vendor_hid")] fn vendor_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; + /// Option to return a firmware version that is shown as device info. fn firmware_version(&self) -> Option { None diff --git a/libraries/opensk/src/env/test/mod.rs b/libraries/opensk/src/env/test/mod.rs index 81d9ca4e..b8a1da6b 100644 --- a/libraries/opensk/src/env/test/mod.rs +++ b/libraries/opensk/src/env/test/mod.rs @@ -34,6 +34,7 @@ pub struct TestEnv { store: Store, customization: TestCustomization, clock: TestClock, + soft_reset: bool, } pub type TestRng = StdRng; @@ -127,6 +128,7 @@ impl Default for TestEnv { store, customization, clock, + soft_reset: false, } } } @@ -139,6 +141,10 @@ impl TestEnv { pub fn seed_rng_from_u64(&mut self, seed: u64) { self.rng = StdRng::seed_from_u64(seed); } + + pub fn set_boots_after_soft_reset(&mut self, value: bool) { + self.soft_reset = value; + } } impl TestUserPresence { @@ -227,6 +233,10 @@ impl Env for TestEnv { self } + fn boots_after_soft_reset(&self) -> bool { + self.soft_reset + } + fn firmware_version(&self) -> Option { Some(0) } @@ -247,4 +257,14 @@ mod test { clock.advance(1); assert!(clock.is_elapsed(&timer)); } + + #[test] + fn test_soft_reset() { + let mut env = TestEnv::default(); + assert!(!env.boots_after_soft_reset()); + env.set_boots_after_soft_reset(true); + assert!(env.boots_after_soft_reset()); + env.set_boots_after_soft_reset(false); + assert!(!env.boots_after_soft_reset()); + } } diff --git a/libraries/opensk/src/lib.rs b/libraries/opensk/src/lib.rs index adbf5ef9..f62aacf4 100644 --- a/libraries/opensk/src/lib.rs +++ b/libraries/opensk/src/lib.rs @@ -63,12 +63,8 @@ impl Ctap { /// Instantiates a CTAP implementation given its environment. // This should only take the environment, but it temporarily takes the boot time until the // clock is part of the environment. - pub fn new(mut env: E, is_soft_reset: bool) -> Self { - let state = if is_soft_reset { - CtapState::::new_soft_reset(&mut env) - } else { - CtapState::::new(&mut env) - }; + pub fn new(mut env: E) -> Self { + let state = CtapState::::new(&mut env); let hid = MainHid::default(); #[cfg(feature = "vendor_hid")] let vendor_hid = VendorHid::default(); @@ -163,7 +159,7 @@ mod test { #[test] fn test_wink() { let env = TestEnv::default(); - let mut ctap = Ctap::::new(env, false); + let mut ctap = Ctap::::new(env); // Send Init, receive Init response and check wink if disabled. let mut init_response = ctap.process_hid_packet(&init_packet(), Transport::MainHid); @@ -182,7 +178,7 @@ mod test { #[test] fn test_locked_channel_id() { let env = TestEnv::default(); - let mut ctap = Ctap::::new(env, false); + let mut ctap = Ctap::::new(env); // Send Init, receive Init response. let mut init_response = ctap.process_hid_packet(&init_packet(), Transport::MainHid); @@ -204,7 +200,7 @@ mod test { #[test] fn test_hard_reset() { let env = TestEnv::default(); - let mut ctap = Ctap::::new(env, false); + let mut ctap = Ctap::::new(env); // Send Init, receive Init response. let mut init_response = ctap.process_hid_packet(&init_packet(), Transport::MainHid); @@ -224,8 +220,9 @@ mod test { #[test] fn test_soft_reset() { - let env = TestEnv::default(); - let mut ctap = Ctap::::new(env, true); + let mut env = TestEnv::default(); + env.set_boots_after_soft_reset(true); + let mut ctap = Ctap::::new(env); // Send Init, receive Init response. let mut init_response = ctap.process_hid_packet(&init_packet(), Transport::MainHid); @@ -246,7 +243,7 @@ mod test { #[test] fn test_env_api() { let env = TestEnv::default(); - let mut ctap = Ctap::::new(env, true); + let mut ctap = Ctap::::new(env); assert_eq!(ctap.env().firmware_version(), Some(0)); } @@ -254,7 +251,7 @@ mod test { #[cfg(feature = "vendor_hid")] fn test_locked_transport() { let env = TestEnv::default(); - let mut ctap = Ctap::::new(env, false); + let mut ctap = Ctap::::new(env); // Send Init, receive Init response. let mut init_response = ctap.process_hid_packet(&init_packet(), Transport::MainHid); @@ -277,7 +274,7 @@ mod test { #[cfg(feature = "with_ctap1")] fn test_ctap1_initial_state() { let env = TestEnv::default(); - let mut ctap = Ctap::::new(env, false); + let mut ctap = Ctap::::new(env); // Granting doesn't work until a CTAP1 request was processed. ctap.u2f_grant_user_presence(); assert!(!ctap.u2f_needs_user_presence()); diff --git a/src/env/tock/mod.rs b/src/env/tock/mod.rs index ffbc1ac5..497d07d8 100644 --- a/src/env/tock/mod.rs +++ b/src/env/tock/mod.rs @@ -452,6 +452,10 @@ impl E commands::process_vendor_command(self, bytes, channel) } + fn boots_after_soft_reset(&self) -> bool { + false + } + fn firmware_version(&self) -> Option { self.upgrade_storage .as_ref() diff --git a/src/main.rs b/src/main.rs index 0676b553..a71c8ff9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -137,7 +137,7 @@ fn main() { } let env = TockEnv::::default(); - let mut ctap = opensk::Ctap::new(env, false); + let mut ctap = opensk::Ctap::new(env); let mut led_counter = 0; let mut led_blink_timer =