diff --git a/src/ctap/config_command.rs b/src/ctap/config_command.rs index 726634c..5e4daf3 100644 --- a/src/ctap/config_command.rs +++ b/src/ctap/config_command.rs @@ -44,7 +44,6 @@ fn process_set_min_pin_length( force_change_pin |= new_min_pin_length > old_length; } if force_change_pin { - // TODO(kaczmarczyck) actually force a PIN change in PinProtocolV1 persistent_store.force_pin_change()?; } persistent_store.set_min_pin_length(new_min_pin_length)?; @@ -248,6 +247,62 @@ mod test { ); } + #[test] + fn test_process_set_min_pin_length_force_pin_change_implicit() { + let mut rng = ThreadRng256 {}; + let mut persistent_store = PersistentStore::new(&mut rng); + let key_agreement_key = crypto::ecdh::SecKey::gensk(&mut rng); + let pin_uv_auth_token = [0x55; 32]; + let mut pin_protocol_v1 = PinProtocolV1::new_test(key_agreement_key, pin_uv_auth_token); + + persistent_store.set_pin(&[0x88; 16], 4).unwrap(); + // Increase min PIN, force PIN change. + let min_pin_length = 6; + let mut config_params = create_min_pin_config_params(min_pin_length, None); + let pin_uv_auth_param = Some(vec![ + 0x81, 0x37, 0x37, 0xF3, 0xD8, 0x69, 0xBD, 0x74, 0xFE, 0x88, 0x30, 0x8C, 0xC4, 0x2E, + 0xA8, 0xC8, + ]); + config_params.pin_uv_auth_param = pin_uv_auth_param; + let config_response = + process_config(&mut persistent_store, &mut pin_protocol_v1, config_params); + assert_eq!(config_response, Ok(ResponseData::AuthenticatorConfig)); + assert_eq!(persistent_store.min_pin_length(), Ok(min_pin_length)); + assert_eq!(persistent_store.has_force_pin_change(), Ok(true)); + } + + #[test] + fn test_process_set_min_pin_length_force_pin_change_explicit() { + let mut rng = ThreadRng256 {}; + let mut persistent_store = PersistentStore::new(&mut rng); + let key_agreement_key = crypto::ecdh::SecKey::gensk(&mut rng); + let pin_uv_auth_token = [0x55; 32]; + let mut pin_protocol_v1 = PinProtocolV1::new_test(key_agreement_key, pin_uv_auth_token); + + persistent_store.set_pin(&[0x88; 16], 4).unwrap(); + let pin_uv_auth_param = Some(vec![ + 0xE3, 0x74, 0xF4, 0x27, 0xBE, 0x7D, 0x40, 0xB5, 0x71, 0xB6, 0xB4, 0x1A, 0xD2, 0xC1, + 0x53, 0xD7, + ]); + let set_min_pin_length_params = SetMinPinLengthParams { + new_min_pin_length: Some(persistent_store.min_pin_length().unwrap()), + min_pin_length_rp_ids: None, + force_change_pin: Some(true), + }; + let config_params = AuthenticatorConfigParameters { + sub_command: ConfigSubCommand::SetMinPinLength, + sub_command_params: Some(ConfigSubCommandParams::SetMinPinLength( + set_min_pin_length_params, + )), + pin_uv_auth_param, + pin_uv_auth_protocol: Some(1), + }; + let config_response = + process_config(&mut persistent_store, &mut pin_protocol_v1, config_params); + assert_eq!(config_response, Ok(ResponseData::AuthenticatorConfig)); + assert_eq!(persistent_store.has_force_pin_change(), Ok(true)); + } + #[test] fn test_process_config_vendor_prototype() { let mut rng = ThreadRng256 {}; diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 6ffd108..6f10589 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -1006,6 +1006,10 @@ where ); options_map.insert(String::from("credMgmt"), true); options_map.insert(String::from("setMinPINLength"), true); + options_map.insert( + String::from("forcePINChange"), + self.persistent_store.has_force_pin_change()?, + ); Ok(ResponseData::AuthenticatorGetInfo( AuthenticatorGetInfoResponse { versions: vec![ @@ -1177,7 +1181,7 @@ mod test { MakeCredentialOptions, PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity, }; use super::*; - use cbor::{cbor_array, cbor_map}; + use cbor::{cbor_array, cbor_array_vec, cbor_map}; use crypto::rng256::ThreadRng256; const CLOCK_FREQUENCY_HZ: usize = 32768; @@ -1233,41 +1237,44 @@ mod test { let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let info_reponse = ctap_state.process_command(&[0x04], DUMMY_CHANNEL_ID, DUMMY_CLOCK_VALUE); - let mut expected_response = vec![0x00, 0xAD, 0x01]; - // The version array differs with CTAP1, always including 2.0 and 2.1. - #[cfg(not(feature = "with_ctap1"))] - let version_count = 2; - #[cfg(feature = "with_ctap1")] - let version_count = 3; - expected_response.push(0x80 + version_count); - #[cfg(feature = "with_ctap1")] - expected_response.extend(&[0x66, 0x55, 0x32, 0x46, 0x5F, 0x56, 0x32]); - expected_response.extend( - [ - 0x68, 0x46, 0x49, 0x44, 0x4F, 0x5F, 0x32, 0x5F, 0x30, 0x6C, 0x46, 0x49, 0x44, 0x4F, - 0x5F, 0x32, 0x5F, 0x31, 0x5F, 0x50, 0x52, 0x45, 0x02, 0x84, 0x6B, 0x68, 0x6D, 0x61, - 0x63, 0x2D, 0x73, 0x65, 0x63, 0x72, 0x65, 0x74, 0x6B, 0x63, 0x72, 0x65, 0x64, 0x50, - 0x72, 0x6F, 0x74, 0x65, 0x63, 0x74, 0x6C, 0x6D, 0x69, 0x6E, 0x50, 0x69, 0x6E, 0x4C, - 0x65, 0x6E, 0x67, 0x74, 0x68, 0x68, 0x63, 0x72, 0x65, 0x64, 0x42, 0x6C, 0x6F, 0x62, - 0x03, 0x50, - ] - .iter(), - ); - expected_response.extend(&ctap_state.persistent_store.aaguid().unwrap()); - expected_response.extend( - [ - 0x04, 0xA5, 0x62, 0x72, 0x6B, 0xF5, 0x62, 0x75, 0x70, 0xF5, 0x68, 0x63, 0x72, 0x65, - 0x64, 0x4D, 0x67, 0x6D, 0x74, 0xF5, 0x69, 0x63, 0x6C, 0x69, 0x65, 0x6E, 0x74, 0x50, - 0x69, 0x6E, 0xF4, 0x6F, 0x73, 0x65, 0x74, 0x4D, 0x69, 0x6E, 0x50, 0x49, 0x4E, 0x4C, - 0x65, 0x6E, 0x67, 0x74, 0x68, 0xF5, 0x05, 0x19, 0x04, 0x00, 0x06, 0x81, 0x01, 0x08, - 0x18, 0x70, 0x09, 0x81, 0x63, 0x75, 0x73, 0x62, 0x0A, 0x81, 0xA2, 0x63, 0x61, 0x6C, - 0x67, 0x26, 0x64, 0x74, 0x79, 0x70, 0x65, 0x6A, 0x70, 0x75, 0x62, 0x6C, 0x69, 0x63, - 0x2D, 0x6B, 0x65, 0x79, 0x0D, 0x04, 0x0F, 0x18, 0x20, 0x10, 0x08, 0x14, 0x18, 0x96, - ] - .iter(), - ); + let expected_cbor = cbor_map_options! { + 0x01 => cbor_array_vec![vec![ + #[cfg(feature = "with_ctap1")] + String::from(U2F_VERSION_STRING), + String::from(FIDO2_VERSION_STRING), + String::from(FIDO2_1_VERSION_STRING), + ]], + 0x02 => cbor_array_vec![vec![ + String::from("hmac-secret"), + String::from("credProtect"), + String::from("minPinLength"), + String::from("credBlob"), + ]], + 0x03 => ctap_state.persistent_store.aaguid().unwrap(), + 0x04 => cbor_map! { + "rk" => true, + "up" => true, + "clientPin" => false, + "credMgmt" => true, + "setMinPINLength" => true, + "forcePINChange" => false, + }, + 0x05 => 1024, + 0x06 => cbor_array_vec![vec![1]], + 0x07 => MAX_CREDENTIAL_COUNT_IN_LIST.map(|c| c as u64), + 0x08 => CREDENTIAL_ID_SIZE as u64, + 0x09 => cbor_array_vec![vec!["usb"]], + 0x0A => cbor_array_vec![vec![ES256_CRED_PARAM]], + 0x0C => DEFAULT_CRED_PROTECT.map(|c| c as u64), + 0x0D => ctap_state.persistent_store.min_pin_length().unwrap() as u64, + 0x0F => MAX_CRED_BLOB_LENGTH as u64, + 0x10 => MAX_RP_IDS_LENGTH as u64, + 0x14 => ctap_state.persistent_store.remaining_credentials().unwrap() as u64, + }; - assert_eq!(info_reponse, expected_response); + let mut response_cbor = vec![0x00]; + assert!(cbor::write(expected_cbor, &mut response_cbor)); + assert_eq!(info_reponse, response_cbor); } fn create_minimal_make_credential_parameters() -> AuthenticatorMakeCredentialParameters { diff --git a/src/ctap/pin_protocol_v1.rs b/src/ctap/pin_protocol_v1.rs index 0e46573..eb537f0 100644 --- a/src/ctap/pin_protocol_v1.rs +++ b/src/ctap/pin_protocol_v1.rs @@ -328,6 +328,10 @@ impl PinProtocolV1 { let token_encryption_key = crypto::aes256::EncryptionKey::new(&shared_secret); let pin_decryption_key = crypto::aes256::DecryptionKey::new(&token_encryption_key); self.verify_pin_hash_enc(rng, persistent_store, &pin_decryption_key, pin_hash_enc)?; + // TODO(kaczmarczyck) can this be moved up in the specification? + if persistent_store.has_force_pin_change()? { + return Err(Ctap2StatusCode::CTAP2_ERR_PIN_INVALID); + } // Assuming PIN_TOKEN_LENGTH % block_size == 0 here. let iv = [0u8; 16]; @@ -821,6 +825,28 @@ mod test { ); } + #[test] + fn test_process_get_pin_token_force_pin_change() { + let mut rng = ThreadRng256 {}; + let mut persistent_store = PersistentStore::new(&mut rng); + set_standard_pin(&mut persistent_store); + assert_eq!(persistent_store.force_pin_change(), Ok(())); + let mut pin_protocol_v1 = PinProtocolV1::new(&mut rng); + let pk = pin_protocol_v1.key_agreement_key.genpk(); + let shared_secret = pin_protocol_v1.key_agreement_key.exchange_x_sha256(&pk); + let key_agreement = CoseKey::from(pk); + let pin_hash_enc = encrypt_standard_pin_hash(&shared_secret); + assert_eq!( + pin_protocol_v1.process_get_pin_token( + &mut rng, + &mut persistent_store, + key_agreement, + pin_hash_enc + ), + Err(Ctap2StatusCode::CTAP2_ERR_PIN_INVALID), + ); + } + #[test] fn test_process_get_pin_uv_auth_token_using_pin_with_permissions() { let mut rng = ThreadRng256 {}; @@ -885,6 +911,30 @@ mod test { ); } + #[test] + fn test_process_get_pin_token_force_pin_change_force_pin_change() { + let mut rng = ThreadRng256 {}; + let mut persistent_store = PersistentStore::new(&mut rng); + set_standard_pin(&mut persistent_store); + assert_eq!(persistent_store.force_pin_change(), Ok(())); + let mut pin_protocol_v1 = PinProtocolV1::new(&mut rng); + let pk = pin_protocol_v1.key_agreement_key.genpk(); + let shared_secret = pin_protocol_v1.key_agreement_key.exchange_x_sha256(&pk); + let key_agreement = CoseKey::from(pk); + let pin_hash_enc = encrypt_standard_pin_hash(&shared_secret); + assert_eq!( + pin_protocol_v1.process_get_pin_uv_auth_token_using_pin_with_permissions( + &mut rng, + &mut persistent_store, + key_agreement, + pin_hash_enc, + 0x03, + Some(String::from("example.com")), + ), + Err(Ctap2StatusCode::CTAP2_ERR_PIN_INVALID), + ); + } + #[test] fn test_process() { let mut rng = ThreadRng256 {}; diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index ffc5dd6..b85f918 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -30,6 +30,7 @@ use arrayref::array_ref; use cbor::cbor_array_vec; use core::convert::TryInto; use crypto::rng256::Rng256; +use persistent_store::StoreUpdate; // Those constants may be modified before compilation to tune the behavior of the key. // @@ -384,7 +385,15 @@ impl PersistentStore { let mut pin_properties = [0; 1 + PIN_AUTH_LENGTH]; pin_properties[0] = pin_code_point_length; pin_properties[1..].clone_from_slice(pin_hash); - Ok(self.store.insert(key::PIN_PROPERTIES, &pin_properties)?) + Ok(self.store.transaction(&[ + StoreUpdate::Insert { + key: key::PIN_PROPERTIES, + value: &pin_properties[..], + }, + StoreUpdate::Remove { + key: key::FORCE_PIN_CHANGE, + }, + ])?) } /// Returns the number of remaining PIN retries. @@ -541,9 +550,18 @@ impl PersistentStore { Ok(()) } + /// Returns whether the PIN needs to be changed before its next usage. + pub fn has_force_pin_change(&self) -> Result { + match self.store.find(key::FORCE_PIN_CHANGE)? { + None => Ok(false), + Some(value) if value.is_empty() => Ok(true), + _ => Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR), + } + } + + /// Marks the PIN as outdated with respect to the new PIN policy. pub fn force_pin_change(&mut self) -> Result<(), Ctap2StatusCode> { - // TODO(kaczmarczyck) implement storage logic - Ok(()) + Ok(self.store.insert(key::FORCE_PIN_CHANGE, &[])?) } } @@ -1148,6 +1166,18 @@ mod test { } } + #[test] + fn test_force_pin_change() { + let mut rng = ThreadRng256 {}; + let mut persistent_store = PersistentStore::new(&mut rng); + + assert!(!persistent_store.has_force_pin_change().unwrap()); + assert_eq!(persistent_store.force_pin_change(), Ok(())); + assert!(persistent_store.has_force_pin_change().unwrap()); + assert_eq!(persistent_store.set_pin(&[0x88; 16], 8), Ok(())); + assert!(!persistent_store.has_force_pin_change().unwrap()); + } + #[test] fn test_serialize_deserialize_credential() { let mut rng = ThreadRng256 {}; diff --git a/src/ctap/storage/key.rs b/src/ctap/storage/key.rs index c6e46e2..1c0e21e 100644 --- a/src/ctap/storage/key.rs +++ b/src/ctap/storage/key.rs @@ -88,6 +88,9 @@ make_partition! { /// board may configure `MAX_SUPPORTED_RESIDENT_KEYS` depending on the storage size. CREDENTIALS = 1700..2000; + /// If this entry exists and equals 1, the PIN needs to be changed. + FORCE_PIN_CHANGE = 2040; + /// The secret of the CredRandom feature. CRED_RANDOM_SECRET = 2041;