From b032a15654711bafcc797c148ed389056f8bc92d Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Fri, 4 Dec 2020 13:17:33 +0100 Subject: [PATCH] makes the global signature counter more privacy friendly --- src/ctap/ctap1.rs | 35 +++++-- src/ctap/mod.rs | 246 ++++++++++++++++++++++++++++++++++++++++---- src/ctap/storage.rs | 31 +++++- 3 files changed, 276 insertions(+), 36 deletions(-) diff --git a/src/ctap/ctap1.rs b/src/ctap/ctap1.rs index 5bc759c..5919431 100644 --- a/src/ctap/ctap1.rs +++ b/src/ctap/ctap1.rs @@ -388,6 +388,7 @@ impl Ctap1Command { mod test { use super::super::{key_material, CREDENTIAL_ID_BASE_SIZE, USE_SIGNATURE_COUNTER}; use super::*; + use byteorder::{BigEndian, ByteOrder}; use crypto::rng256::ThreadRng256; use crypto::Hash256; @@ -642,6 +643,16 @@ mod test { assert_eq!(response, Err(Ctap1StatusCode::SW_WRONG_DATA)); } + fn check_signature_counter(response: &[u8; 4], signature_counter: u32) { + if USE_SIGNATURE_COUNTER { + let mut signature_counter_bytes = [0u8; 4]; + BigEndian::write_u32(&mut signature_counter_bytes, signature_counter); + assert_eq!(response, &signature_counter_bytes); + } else { + assert_eq!(response, &[0x00, 0x00, 0x00, 0x00]); + } + } + #[test] fn test_process_authenticate_enforce() { let mut rng = ThreadRng256 {}; @@ -662,11 +673,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 +703,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..56cca9a 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(()) } @@ -1094,9 +1097,43 @@ mod test { // The expected response is split to only assert the non-random parts. assert_eq!(fmt, "packed"); 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, + 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, + INITIAL_SIGNATURE_COUNTER as u8, ]; expected_auth_data.extend(&ctap_state.persistent_store.aaguid().unwrap()); expected_auth_data.extend(&[0x00, 0x20]); @@ -1131,9 +1168,43 @@ mod test { // The expected response is split to only assert the non-random parts. assert_eq!(fmt, "packed"); 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, + 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, + 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]); @@ -1280,9 +1351,43 @@ mod test { // The expected response is split to only assert the non-random parts. assert_eq!(fmt, "packed"); 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, + 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, + 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]); @@ -1329,9 +1434,43 @@ mod test { // The expected response is split to only assert the non-random parts. assert_eq!(fmt, "packed"); 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, + 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, + INITIAL_SIGNATURE_COUNTER as u8, ]; expected_auth_data.extend(&ctap_state.persistent_store.aaguid().unwrap()); expected_auth_data.extend(&[0x00, 0x20]); @@ -1374,6 +1513,7 @@ mod test { response: Result, expected_user: PublicKeyCredentialUserEntity, flags: u8, + signature_counter: u32, expected_number_of_credentials: Option, ) { match response.unwrap() { @@ -1384,11 +1524,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 +1545,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 +1558,7 @@ mod test { response, expected_user, 0x00, + signature_counter, expected_number_of_credentials, ); } @@ -1444,7 +1591,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 +1788,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 +1895,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 +1971,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 +2222,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 {};