diff --git a/src/ctap/apdu.rs b/src/ctap/apdu.rs index b39d7fb..949151e 100644 --- a/src/ctap/apdu.rs +++ b/src/ctap/apdu.rs @@ -226,7 +226,7 @@ impl TryFrom<&[u8]> for APDU { } } - return Err(ApduStatusCode::SW_INTERNAL_EXCEPTION); + Err(ApduStatusCode::SW_INTERNAL_EXCEPTION) } } diff --git a/src/ctap/ctap1.rs b/src/ctap/ctap1.rs index 5bc759c..e434c36 100644 --- a/src/ctap/ctap1.rs +++ b/src/ctap/ctap1.rs @@ -642,6 +642,14 @@ mod test { assert_eq!(response, Err(Ctap1StatusCode::SW_WRONG_DATA)); } + fn check_signature_counter(response: &[u8; 4], signature_counter: u32) { + if USE_SIGNATURE_COUNTER { + assert_eq!(u32::from_be_bytes(*response), signature_counter); + } else { + assert_eq!(response, &[0x00, 0x00, 0x00, 0x00]); + } + } + #[test] fn test_process_authenticate_enforce() { let mut rng = ThreadRng256 {}; @@ -662,11 +670,13 @@ mod test { let response = Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE).unwrap(); assert_eq!(response[0], 0x01); - if USE_SIGNATURE_COUNTER { - assert_eq!(response[1..5], [0x00, 0x00, 0x00, 0x01]); - } else { - assert_eq!(response[1..5], [0x00, 0x00, 0x00, 0x00]); - } + check_signature_counter( + array_ref!(response, 1, 4), + ctap_state + .persistent_store + .global_signature_counter() + .unwrap(), + ); } #[test] @@ -690,11 +700,13 @@ mod test { let response = Ctap1Command::process_command(&message, &mut ctap_state, TIMEOUT_CLOCK_VALUE).unwrap(); assert_eq!(response[0], 0x01); - if USE_SIGNATURE_COUNTER { - assert_eq!(response[1..5], [0x00, 0x00, 0x00, 0x01]); - } else { - assert_eq!(response[1..5], [0x00, 0x00, 0x00, 0x00]); - } + check_signature_counter( + array_ref!(response, 1, 4), + ctap_state + .persistent_store + .global_signature_counter() + .unwrap(), + ); } #[test] diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index d67796b..ea42caf 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -81,6 +81,7 @@ const USE_BATCH_ATTESTATION: bool = false; // need a flash storage friendly way to implement this feature. The implemented // solution is a compromise to be compatible with U2F and not wasting storage. const USE_SIGNATURE_COUNTER: bool = true; +pub const INITIAL_SIGNATURE_COUNTER: u32 = 1; // Our credential ID consists of // - 16 byte initialization vector for AES-256, // - 32 byte ECDSA private key for the credential, @@ -215,7 +216,9 @@ where pub fn increment_global_signature_counter(&mut self) -> Result<(), Ctap2StatusCode> { if USE_SIGNATURE_COUNTER { - self.persistent_store.incr_global_signature_counter()?; + let increment = self.rng.gen_uniform_u32x8()[0] % 8 + 1; + self.persistent_store + .incr_global_signature_counter(increment)?; } Ok(()) } @@ -1096,8 +1099,9 @@ mod test { let mut expected_auth_data = vec![ 0xA3, 0x79, 0xA6, 0xF6, 0xEE, 0xAF, 0xB9, 0xA5, 0x5E, 0x37, 0x8C, 0x11, 0x80, 0x34, 0xE2, 0x75, 0x1E, 0x68, 0x2F, 0xAB, 0x9F, 0x2D, 0x30, 0xAB, 0x13, 0xD2, - 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0x41, 0x00, 0x00, 0x00, 0x00, + 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0x41, 0x00, 0x00, 0x00, ]; + expected_auth_data.push(INITIAL_SIGNATURE_COUNTER as u8); expected_auth_data.extend(&ctap_state.persistent_store.aaguid().unwrap()); expected_auth_data.extend(&[0x00, 0x20]); assert_eq!( @@ -1133,8 +1137,9 @@ mod test { let mut expected_auth_data = vec![ 0xA3, 0x79, 0xA6, 0xF6, 0xEE, 0xAF, 0xB9, 0xA5, 0x5E, 0x37, 0x8C, 0x11, 0x80, 0x34, 0xE2, 0x75, 0x1E, 0x68, 0x2F, 0xAB, 0x9F, 0x2D, 0x30, 0xAB, 0x13, 0xD2, - 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0x41, 0x00, 0x00, 0x00, 0x00, + 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0x41, 0x00, 0x00, 0x00, ]; + expected_auth_data.push(INITIAL_SIGNATURE_COUNTER as u8); expected_auth_data.extend(&ctap_state.persistent_store.aaguid().unwrap()); expected_auth_data.extend(&[0x00, CREDENTIAL_ID_BASE_SIZE as u8]); assert_eq!( @@ -1282,8 +1287,9 @@ mod test { let mut expected_auth_data = vec![ 0xA3, 0x79, 0xA6, 0xF6, 0xEE, 0xAF, 0xB9, 0xA5, 0x5E, 0x37, 0x8C, 0x11, 0x80, 0x34, 0xE2, 0x75, 0x1E, 0x68, 0x2F, 0xAB, 0x9F, 0x2D, 0x30, 0xAB, 0x13, 0xD2, - 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0xC1, 0x00, 0x00, 0x00, 0x00, + 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0xC1, 0x00, 0x00, 0x00, ]; + expected_auth_data.push(INITIAL_SIGNATURE_COUNTER as u8); expected_auth_data.extend(&ctap_state.persistent_store.aaguid().unwrap()); expected_auth_data.extend(&[0x00, CREDENTIAL_ID_MAX_SIZE as u8]); assert_eq!( @@ -1331,8 +1337,9 @@ mod test { let mut expected_auth_data = vec![ 0xA3, 0x79, 0xA6, 0xF6, 0xEE, 0xAF, 0xB9, 0xA5, 0x5E, 0x37, 0x8C, 0x11, 0x80, 0x34, 0xE2, 0x75, 0x1E, 0x68, 0x2F, 0xAB, 0x9F, 0x2D, 0x30, 0xAB, 0x13, 0xD2, - 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0xC1, 0x00, 0x00, 0x00, 0x00, + 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0xC1, 0x00, 0x00, 0x00, ]; + expected_auth_data.push(INITIAL_SIGNATURE_COUNTER as u8); expected_auth_data.extend(&ctap_state.persistent_store.aaguid().unwrap()); expected_auth_data.extend(&[0x00, 0x20]); assert_eq!( @@ -1374,6 +1381,7 @@ mod test { response: Result, expected_user: PublicKeyCredentialUserEntity, flags: u8, + signature_counter: u32, expected_number_of_credentials: Option, ) { match response.unwrap() { @@ -1384,11 +1392,16 @@ mod test { number_of_credentials, .. } = get_assertion_response; - let expected_auth_data = vec![ + let mut expected_auth_data = vec![ 0xA3, 0x79, 0xA6, 0xF6, 0xEE, 0xAF, 0xB9, 0xA5, 0x5E, 0x37, 0x8C, 0x11, 0x80, 0x34, 0xE2, 0x75, 0x1E, 0x68, 0x2F, 0xAB, 0x9F, 0x2D, 0x30, 0xAB, 0x13, 0xD2, - 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, flags, 0x00, 0x00, 0x00, 0x01, + 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, flags, 0x00, 0x00, 0x00, 0x00, ]; + let signature_counter_position = expected_auth_data.len() - 4; + BigEndian::write_u32( + &mut expected_auth_data[signature_counter_position..], + signature_counter, + ); assert_eq!(auth_data, expected_auth_data); assert_eq!(user, Some(expected_user)); assert_eq!(number_of_credentials, expected_number_of_credentials); @@ -1400,6 +1413,7 @@ mod test { fn check_assertion_response( response: Result, expected_user_id: Vec, + signature_counter: u32, expected_number_of_credentials: Option, ) { let expected_user = PublicKeyCredentialUserEntity { @@ -1412,6 +1426,7 @@ mod test { response, expected_user, 0x00, + signature_counter, expected_number_of_credentials, ); } @@ -1444,7 +1459,11 @@ mod test { DUMMY_CHANNEL_ID, DUMMY_CLOCK_VALUE, ); - check_assertion_response(get_assertion_response, vec![0x1D], None); + let signature_counter = ctap_state + .persistent_store + .global_signature_counter() + .unwrap(); + check_assertion_response(get_assertion_response, vec![0x1D], signature_counter, None); } #[test] @@ -1637,7 +1656,11 @@ mod test { DUMMY_CHANNEL_ID, DUMMY_CLOCK_VALUE, ); - check_assertion_response(get_assertion_response, vec![0x1D], None); + let signature_counter = ctap_state + .persistent_store + .global_signature_counter() + .unwrap(); + check_assertion_response(get_assertion_response, vec![0x1D], signature_counter, None); let credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, @@ -1740,10 +1763,26 @@ mod test { DUMMY_CHANNEL_ID, DUMMY_CLOCK_VALUE, ); - check_assertion_response_with_user(get_assertion_response, user2, 0x04, Some(2)); + let signature_counter = ctap_state + .persistent_store + .global_signature_counter() + .unwrap(); + check_assertion_response_with_user( + get_assertion_response, + user2, + 0x04, + signature_counter, + Some(2), + ); let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE); - check_assertion_response_with_user(get_assertion_response, user1, 0x04, None); + check_assertion_response_with_user( + get_assertion_response, + user1, + 0x04, + signature_counter, + None, + ); let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE); assert_eq!( @@ -1800,13 +1839,22 @@ mod test { DUMMY_CHANNEL_ID, DUMMY_CLOCK_VALUE, ); - check_assertion_response(get_assertion_response, vec![0x03], Some(3)); + let signature_counter = ctap_state + .persistent_store + .global_signature_counter() + .unwrap(); + check_assertion_response( + get_assertion_response, + vec![0x03], + signature_counter, + Some(3), + ); let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE); - check_assertion_response(get_assertion_response, vec![0x02], None); + check_assertion_response(get_assertion_response, vec![0x02], signature_counter, None); let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE); - check_assertion_response(get_assertion_response, vec![0x01], None); + check_assertion_response(get_assertion_response, vec![0x01], signature_counter, None); let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE); assert_eq!( @@ -2042,4 +2090,26 @@ mod test { .is_none()); } } + + #[test] + fn test_signature_counter() { + let mut rng = ThreadRng256 {}; + let user_immediately_present = |_| Ok(()); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); + + let mut last_counter = ctap_state + .persistent_store + .global_signature_counter() + .unwrap(); + assert!(last_counter > 0); + for _ in 0..100 { + assert!(ctap_state.increment_global_signature_counter().is_ok()); + let next_counter = ctap_state + .persistent_store + .global_signature_counter() + .unwrap(); + assert!(next_counter > last_counter); + last_counter = next_counter; + } + } } diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 0e4941a..911bdd7 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -18,6 +18,7 @@ use crate::ctap::data_formats::{CredentialProtectionPolicy, PublicKeyCredentialS use crate::ctap::key_material; use crate::ctap::pin_protocol_v1::PIN_AUTH_LENGTH; use crate::ctap::status_code::Ctap2StatusCode; +use crate::ctap::INITIAL_SIGNATURE_COUNTER; use crate::embedded_flash::{self, StoreConfig, StoreEntry, StoreError}; use alloc::string::String; #[cfg(any(test, feature = "ram_storage", feature = "with_ctap2_1"))] @@ -366,16 +367,16 @@ impl PersistentStore { Ok(self .store .find_one(&Key::GlobalSignatureCounter) - .map_or(0, |(_, entry)| { + .map_or(INITIAL_SIGNATURE_COUNTER, |(_, entry)| { u32::from_ne_bytes(*array_ref!(entry.data, 0, 4)) })) } - pub fn incr_global_signature_counter(&mut self) -> Result<(), Ctap2StatusCode> { + pub fn incr_global_signature_counter(&mut self, increment: u32) -> Result<(), Ctap2StatusCode> { let mut buffer = [0; core::mem::size_of::()]; match self.store.find_one(&Key::GlobalSignatureCounter) { None => { - buffer.copy_from_slice(&1u32.to_ne_bytes()); + buffer.copy_from_slice(&(increment + INITIAL_SIGNATURE_COUNTER).to_ne_bytes()); self.store.insert(StoreEntry { tag: GLOBAL_SIGNATURE_COUNTER, data: &buffer, @@ -385,7 +386,7 @@ impl PersistentStore { Some((index, entry)) => { let value = u32::from_ne_bytes(*array_ref!(entry.data, 0, 4)); // In hopes that servers handle the wrapping gracefully. - buffer.copy_from_slice(&value.wrapping_add(1).to_ne_bytes()); + buffer.copy_from_slice(&value.wrapping_add(increment).to_ne_bytes()); self.store.replace( index, StoreEntry { @@ -1136,6 +1137,28 @@ mod test { assert_eq!(persistent_store._min_pin_length_rp_ids().unwrap(), rp_ids); } + #[test] + fn test_global_signature_counter() { + let mut rng = ThreadRng256 {}; + let mut persistent_store = PersistentStore::new(&mut rng); + + let mut counter_value = 1; + assert_eq!( + persistent_store.global_signature_counter().unwrap(), + counter_value + ); + for increment in 1..10 { + assert!(persistent_store + .incr_global_signature_counter(increment) + .is_ok()); + counter_value += increment; + assert_eq!( + persistent_store.global_signature_counter().unwrap(), + counter_value + ); + } + } + #[test] fn test_serialize_deserialize_credential() { let mut rng = ThreadRng256 {};