Merge "Add a new permission check around unique id attestation"

2 files changed, 24 insertions, 8 deletions

diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index 1f6be32b..28de1ec8 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -27,7 +27,8 @@ use crate::metrics_store::log_key_creation_event_stats;
 use crate::remote_provisioning::RemProvState;
 use crate::super_key::{KeyBlob, SuperKeyManager};
 use crate::utils::{
-    check_device_attestation_permissions, check_key_permission, is_device_id_attestation_tag,
+    check_device_attestation_permissions, check_key_permission,
+    check_unique_id_attestation_permissions, is_device_id_attestation_tag,
     key_characteristics_to_internal, uid_to_android_user, watchdog as wd,
 };
 use crate::{
@@ -452,10 +453,14 @@ impl KeystoreSecurityLevel {
         }
 
         if params.iter().any(|kp| kp.tag == Tag::INCLUDE_UNIQUE_ID) {
-            check_key_permission(KeyPerm::GenUniqueId, key, &None).context(concat!(
-                "In add_required_parameters: ",
-                "Caller does not have the permission to generate a unique ID"
-            ))?;
+            if check_key_permission(KeyPerm::GenUniqueId, key, &None).is_err()
+                && check_unique_id_attestation_permissions().is_err()
+            {
+                return Err(Error::perm()).context(
+                    "In add_required_parameters: \
+                    Caller does not have the permission to generate a unique ID",
+                );
+            }
             if self.id_rotation_state.had_factory_reset_since_id_rotation().context(
                 "In add_required_parameters: Call to had_factory_reset_since_id_rotation failed.",
             )? {
diff --git a/keystore2/src/utils.rs b/keystore2/src/utils.rs
index a312c4b8..9db2eb9d 100644
--- a/keystore2/src/utils.rs
+++ b/keystore2/src/utils.rs
@@ -107,9 +107,20 @@ pub fn is_device_id_attestation_tag(tag: Tag) -> bool {
 }
 
 /// This function checks whether the calling app has the Android permissions needed to attest device
-/// identifiers. It throws an error if the permissions cannot be verified, or if the caller doesn't
-/// have the right permissions, and returns silently otherwise.
+/// identifiers. It throws an error if the permissions cannot be verified or if the caller doesn't
+/// have the right permissions. Otherwise it returns silently.
 pub fn check_device_attestation_permissions() -> anyhow::Result<()> {
+    check_android_permission("android.permission.READ_PRIVILEGED_PHONE_STATE")
+}
+
+/// This function checks whether the calling app has the Android permissions needed to attest the
+/// device-unique identifier. It throws an error if the permissions cannot be verified or if the
+/// caller doesn't have the right permissions. Otherwise it returns silently.
+pub fn check_unique_id_attestation_permissions() -> anyhow::Result<()> {
+    check_android_permission("android.permission.REQUEST_UNIQUE_ID_ATTESTATION")
+}
+
+fn check_android_permission(permission: &str) -> anyhow::Result<()> {
     let permission_controller: Strong<dyn IPermissionController::IPermissionController> =
         binder::get_interface("permission")?;
 
@@ -119,7 +130,7 @@ pub fn check_device_attestation_permissions() -> anyhow::Result<()> {
             500,
         );
         permission_controller.checkPermission(
-            "android.permission.READ_PRIVILEGED_PHONE_STATE",
+            permission,
             ThreadState::get_calling_pid(),
             ThreadState::get_calling_uid() as i32,
         )