diff --git a/src/ctap/credential_management.rs b/src/ctap/credential_management.rs index acc9278..b4ecf6f 100644 --- a/src/ctap/credential_management.rs +++ b/src/ctap/credential_management.rs @@ -106,6 +106,22 @@ fn enumerate_credentials_response( }) } +/// Check if the token permissions have the correct associated RP ID. +/// +/// Either no RP ID is associated, or the RP ID matches the stored credential. +fn check_rp_id_permissions( + persistent_store: &mut PersistentStore, + pin_protocol_v1: &mut PinProtocolV1, + credential_id: &[u8], +) -> Result<(), Ctap2StatusCode> { + // Pre-check a sufficient condition before calling the store. + if pin_protocol_v1.has_no_rp_id_permission().is_ok() { + return Ok(()); + } + let (_, credential) = persistent_store.find_credential_item(credential_id)?; + pin_protocol_v1.has_no_or_rp_id_permission(&credential.rp_id) +} + /// Processes the subcommand getCredsMetadata for CredentialManagement. fn process_get_creds_metadata( persistent_store: &PersistentStore, @@ -155,12 +171,14 @@ fn process_enumerate_rps_get_next_rp( fn process_enumerate_credentials_begin( persistent_store: &PersistentStore, stateful_command_permission: &mut StatefulPermission, + pin_protocol_v1: &mut PinProtocolV1, sub_command_params: CredentialManagementSubCommandParameters, now: ClockValue, ) -> Result { let rp_id_hash = sub_command_params .rp_id_hash .ok_or(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)?; + pin_protocol_v1.has_no_or_rp_id_hash_permission(&rp_id_hash[..])?; let mut iter_result = Ok(()); let iter = persistent_store.iter_credentials(&mut iter_result)?; let mut rp_credentials: Vec = iter @@ -199,18 +217,21 @@ fn process_enumerate_credentials_get_next_credential( /// Processes the subcommand deleteCredential for CredentialManagement. fn process_delete_credential( persistent_store: &mut PersistentStore, + pin_protocol_v1: &mut PinProtocolV1, sub_command_params: CredentialManagementSubCommandParameters, ) -> Result<(), Ctap2StatusCode> { let credential_id = sub_command_params .credential_id .ok_or(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)? .key_id; + check_rp_id_permissions(persistent_store, pin_protocol_v1, &credential_id)?; persistent_store.delete_credential(&credential_id) } /// Processes the subcommand updateUserInformation for CredentialManagement. fn process_update_user_information( persistent_store: &mut PersistentStore, + pin_protocol_v1: &mut PinProtocolV1, sub_command_params: CredentialManagementSubCommandParameters, ) -> Result<(), Ctap2StatusCode> { let credential_id = sub_command_params @@ -220,6 +241,7 @@ fn process_update_user_information( let user = sub_command_params .user .ok_or(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)?; + check_rp_id_permissions(persistent_store, pin_protocol_v1, &credential_id)?; persistent_store.update_credential(&credential_id, user) } @@ -255,13 +277,10 @@ pub fn process_credential_management( match sub_command { CredentialManagementSubCommand::GetCredsMetadata | CredentialManagementSubCommand::EnumerateRpsBegin - | CredentialManagementSubCommand::DeleteCredential | CredentialManagementSubCommand::EnumerateCredentialsBegin + | CredentialManagementSubCommand::DeleteCredential | CredentialManagementSubCommand::UpdateUserInformation => { check_pin_uv_auth_protocol(pin_protocol)?; - persistent_store - .pin_hash()? - .ok_or(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED)?; let pin_auth = pin_auth.ok_or(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED)?; let mut management_data = vec![sub_command as u8]; if let Some(sub_command_params) = sub_command_params.clone() { @@ -272,9 +291,8 @@ pub fn process_credential_management( if !pin_protocol_v1.verify_pin_auth_token(&management_data, &pin_auth) { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID); } + // The RP ID permission is handled differently per subcommand below. pin_protocol_v1.has_permission(PinPermission::CredentialManagement)?; - pin_protocol_v1.has_no_permission_rp_id()?; - // TODO(kaczmarczyck) sometimes allow a RP ID } CredentialManagementSubCommand::EnumerateRpsGetNextRp | CredentialManagementSubCommand::EnumerateCredentialsGetNextCredential => {} @@ -282,13 +300,17 @@ pub fn process_credential_management( let response = match sub_command { CredentialManagementSubCommand::GetCredsMetadata => { + pin_protocol_v1.has_no_rp_id_permission()?; Some(process_get_creds_metadata(persistent_store)?) } - CredentialManagementSubCommand::EnumerateRpsBegin => Some(process_enumerate_rps_begin( - persistent_store, - stateful_command_permission, - now, - )?), + CredentialManagementSubCommand::EnumerateRpsBegin => { + pin_protocol_v1.has_no_rp_id_permission()?; + Some(process_enumerate_rps_begin( + persistent_store, + stateful_command_permission, + now, + )?) + } CredentialManagementSubCommand::EnumerateRpsGetNextRp => Some( process_enumerate_rps_get_next_rp(persistent_store, stateful_command_permission)?, ), @@ -296,6 +318,7 @@ pub fn process_credential_management( Some(process_enumerate_credentials_begin( persistent_store, stateful_command_permission, + pin_protocol_v1, sub_command_params.ok_or(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)?, now, )?) @@ -309,6 +332,7 @@ pub fn process_credential_management( CredentialManagementSubCommand::DeleteCredential => { process_delete_credential( persistent_store, + pin_protocol_v1, sub_command_params.ok_or(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)?, )?; None @@ -316,6 +340,7 @@ pub fn process_credential_management( CredentialManagementSubCommand::UpdateUserInformation => { process_update_user_information( persistent_store, + pin_protocol_v1, sub_command_params.ok_or(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)?, )?; None diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index ab66177..0579f62 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -638,7 +638,7 @@ where } self.pin_protocol_v1 .has_permission(PinPermission::MakeCredential)?; - self.pin_protocol_v1.has_permission_for_rp_id(&rp_id)?; + self.pin_protocol_v1.require_rp_id_permission(&rp_id)?; UP_FLAG | UV_FLAG | AT_FLAG | ed_flag } None => { @@ -923,7 +923,7 @@ where } self.pin_protocol_v1 .has_permission(PinPermission::GetAssertion)?; - self.pin_protocol_v1.has_permission_for_rp_id(&rp_id)?; + self.pin_protocol_v1.require_rp_id_permission(&rp_id)?; UV_FLAG } None => { diff --git a/src/ctap/pin_protocol_v1.rs b/src/ctap/pin_protocol_v1.rs index 6ec5644..d95b5d1 100644 --- a/src/ctap/pin_protocol_v1.rs +++ b/src/ctap/pin_protocol_v1.rs @@ -501,6 +501,7 @@ impl PinProtocolV1 { encrypt_hmac_secret_output(&shared_secret, &salt_enc[..], cred_random) } + /// Check if the required command's token permission is granted. pub fn has_permission(&self, permission: PinPermission) -> Result<(), Ctap2StatusCode> { // Relies on the fact that all permissions are represented by powers of two. if permission as u8 & self.permissions != 0 { @@ -510,22 +511,47 @@ impl PinProtocolV1 { } } - pub fn has_no_permission_rp_id(&self) -> Result<(), Ctap2StatusCode> { + /// Check if no RP ID is associated with the token permission. + pub fn has_no_rp_id_permission(&self) -> Result<(), Ctap2StatusCode> { if self.permissions_rp_id.is_some() { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID); } Ok(()) } - pub fn has_permission_for_rp_id(&mut self, rp_id: &str) -> Result<(), Ctap2StatusCode> { - if let Some(permissions_rp_id) = &self.permissions_rp_id { - if rp_id != permissions_rp_id { - return Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID); - } - } else { - self.permissions_rp_id = Some(String::from(rp_id)); + /// Check if no or the passed RP ID is associated with the token permission. + pub fn has_no_or_rp_id_permission(&mut self, rp_id: &str) -> Result<(), Ctap2StatusCode> { + match &self.permissions_rp_id { + Some(p) if rp_id != p => Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID), + _ => Ok(()), + } + } + + /// Check if no RP ID is associated with the token permission, or it matches the hash. + pub fn has_no_or_rp_id_hash_permission( + &self, + rp_id_hash: &[u8], + ) -> Result<(), Ctap2StatusCode> { + match &self.permissions_rp_id { + Some(p) if rp_id_hash != Sha256::hash(p.as_bytes()) => { + Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID) + } + _ => Ok(()), + } + } + + /// Check if the passed RP ID is associated with the token permission. + /// + /// If no RP ID is associated, associate the passed RP ID as a side effect. + pub fn require_rp_id_permission(&mut self, rp_id: &str) -> Result<(), Ctap2StatusCode> { + match &self.permissions_rp_id { + Some(p) if rp_id != p => Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID), + None => { + self.permissions_rp_id = Some(String::from(rp_id)); + Ok(()) + } + _ => Ok(()), } - Ok(()) } #[cfg(test)] @@ -1150,24 +1176,65 @@ mod test { } #[test] - fn test_has_no_permission_rp_id() { + fn test_has_no_rp_id_permission() { let mut rng = ThreadRng256 {}; let mut pin_protocol_v1 = PinProtocolV1::new(&mut rng); - assert_eq!(pin_protocol_v1.has_no_permission_rp_id(), Ok(())); - assert_eq!(pin_protocol_v1.permissions_rp_id, None,); + assert_eq!(pin_protocol_v1.has_no_rp_id_permission(), Ok(())); + assert_eq!(pin_protocol_v1.permissions_rp_id, None); pin_protocol_v1.permissions_rp_id = Some("example.com".to_string()); assert_eq!( - pin_protocol_v1.has_no_permission_rp_id(), + pin_protocol_v1.has_no_rp_id_permission(), Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID) ); } #[test] - fn test_has_permission_for_rp_id() { + fn test_has_no_or_rp_id_permission() { let mut rng = ThreadRng256 {}; let mut pin_protocol_v1 = PinProtocolV1::new(&mut rng); assert_eq!( - pin_protocol_v1.has_permission_for_rp_id("example.com"), + pin_protocol_v1.has_no_or_rp_id_permission("example.com"), + Ok(()) + ); + assert_eq!(pin_protocol_v1.permissions_rp_id, None); + pin_protocol_v1.permissions_rp_id = Some("example.com".to_string()); + assert_eq!( + pin_protocol_v1.has_no_or_rp_id_permission("example.com"), + Ok(()) + ); + assert_eq!( + pin_protocol_v1.has_no_or_rp_id_permission("another.example.com"), + Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID) + ); + } + + #[test] + fn test_has_no_or_rp_id_hash_permission() { + let mut rng = ThreadRng256 {}; + let mut pin_protocol_v1 = PinProtocolV1::new(&mut rng); + let rp_id_hash = Sha256::hash(b"example.com"); + assert_eq!( + pin_protocol_v1.has_no_or_rp_id_hash_permission(&rp_id_hash), + Ok(()) + ); + assert_eq!(pin_protocol_v1.permissions_rp_id, None); + pin_protocol_v1.permissions_rp_id = Some("example.com".to_string()); + assert_eq!( + pin_protocol_v1.has_no_or_rp_id_hash_permission(&rp_id_hash), + Ok(()) + ); + assert_eq!( + pin_protocol_v1.has_no_or_rp_id_hash_permission(&[0x4A; 32]), + Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID) + ); + } + + #[test] + fn test_require_rp_id_permission() { + let mut rng = ThreadRng256 {}; + let mut pin_protocol_v1 = PinProtocolV1::new(&mut rng); + assert_eq!( + pin_protocol_v1.require_rp_id_permission("example.com"), Ok(()) ); assert_eq!( @@ -1175,11 +1242,11 @@ mod test { Some(String::from("example.com")) ); assert_eq!( - pin_protocol_v1.has_permission_for_rp_id("example.com"), + pin_protocol_v1.require_rp_id_permission("example.com"), Ok(()) ); assert_eq!( - pin_protocol_v1.has_permission_for_rp_id("counter-example.com"), + pin_protocol_v1.require_rp_id_permission("counter-example.com"), Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID) ); } diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index b982922..6231e3c 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -151,7 +151,7 @@ impl PersistentStore { /// # Errors /// /// Returns `CTAP2_ERR_NO_CREDENTIALS` if the credential is not found. - fn find_credential_item( + pub fn find_credential_item( &self, credential_id: &[u8], ) -> Result<(usize, PublicKeyCredentialSource), Ctap2StatusCode> {