commit 66d9e908bf4d5495fbcd17024c5c9fd37f17891f
author Seth Moore <sethmo@google.com> Wed Mar 16 17:20:31 2022 -0700
committer Seth Moore <sethmo@google.com> Wed Mar 30 11:18:00 2022 -0700
tree a10149c14f246c19e0b5efc3685ae12e8b5e1c76
parent 8945ea98e902ed8a56411284c196bb87d8c14920

Add a new permission check around unique id attestation

Allow an application to either have an selinux permission or a
privileged application permission to request a unique id attestation.
Eventually, we will remove the sepolicy permission. However, until
GMSCore is updated to pick up the new permission, keep the policy
around.

Test: KeyAttestationTest
Bug: 216778747
Change-Id: I2401ca630d1599f0399b47e1fd9dd88d78318b95

keystore2/src/security_level.rs

diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index 1f6be32..28de1ec 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -27,7 +27,8 @@
 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 @@
         }
 
         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.",
             )? {

keystore2/src/utils.rs

diff --git a/keystore2/src/utils.rs b/keystore2/src/utils.rs
index a312c4b..9db2eb9 100644
--- a/keystore2/src/utils.rs
+++ b/keystore2/src/utils.rs
@@ -107,9 +107,20 @@
 }
 
 /// 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 @@
             500,
         );
         permission_controller.checkPermission(
-            "android.permission.READ_PRIVILEGED_PHONE_STATE",
+            permission,
             ThreadState::get_calling_pid(),
             ThreadState::get_calling_uid() as i32,
         )