allow RP ID permissions for some subcommands

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<AuthenticatorCredentialManagementResponse, Ctap2StatusCode> {
     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<usize> = 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(
+        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

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 => {

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,23 +511,48 @@ 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> {
+    /// Check if no or the passed RP ID is associated with the token permission.
-        if let Some(permissions_rp_id) = &self.permissions_rp_id {
+    pub fn has_no_or_rp_id_permission(&mut self, rp_id: &str) -> Result<(), Ctap2StatusCode> {
-            if rp_id != permissions_rp_id {
+        match &self.permissions_rp_id {
-                return Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID);
+            Some(p) if rp_id != p => Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID),
+            _ => Ok(()),
         }
-        } else {
+    }
+
+    /// 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(()),
+        }
+    }
 
     #[cfg(test)]
     pub fn new_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.has_no_rp_id_permission(), Ok(()));
-        assert_eq!(pin_protocol_v1.permissions_rp_id, None,);
+        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)
         );
     }

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> {