Correcting permission check for App UIDs listing

Correct the permission check for the Keystore maintenance method
that returns the list of app UIDs which have keys that are
bound to a specific SID.

The previous check relied on SELinux policies. But the Settings
app that calls this method has a permission - MANAGE_USERS -
that is more appropriate to check.

Bug: 302109605
Test: Manual.
Change-Id: Ia26256cf995d16d03d0bb92d8b237f7bbea30d07

3 files changed, 16 insertions, 4 deletions

diff --git a/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl b/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl
index e612db98..abea9582 100644
--- a/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl
+++ b/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl
@@ -151,7 +151,8 @@ interface IKeystoreMaintenance {
      * (addition of a fingerprint, for example), authentication-bound keys may be invalidated.
      * This method allows the platform to find out which apps would be affected (for a given user)
      * when a given user secure ID is removed.
-     * Callers require 'ChangeUser' permission.
+     * Callers require the `android.permission.MANAGE_USERS` Android permission
+     * (not SELinux policy).
      *
      * @param userId The affected user.
      * @param sid The user secure ID - identifier of the authentication method.
diff --git a/keystore2/src/maintenance.rs b/keystore2/src/maintenance.rs
index 8c0ac487..ae988883 100644
--- a/keystore2/src/maintenance.rs
+++ b/keystore2/src/maintenance.rs
@@ -24,7 +24,8 @@ use crate::ks_err;
 use crate::permission::{KeyPerm, KeystorePerm};
 use crate::super_key::{SuperKeyManager, UserState};
 use crate::utils::{
-    check_key_permission, check_keystore_permission, uid_to_android_user, watchdog as wd,
+    check_get_app_uids_affected_by_sid_permissions, check_key_permission,
+    check_keystore_permission, uid_to_android_user, watchdog as wd,
 };
 use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{
     IKeyMintDevice::IKeyMintDevice, SecurityLevel::SecurityLevel,
@@ -292,8 +293,9 @@ impl Maintenance {
         secure_user_id: i64,
     ) -> Result<std::vec::Vec<i64>> {
         // This method is intended to be called by Settings and discloses a list of apps
-        // associated with a user, so it requires the ChangeUser permission.
-        check_keystore_permission(KeystorePerm::ChangeUser).context(ks_err!())?;
+        // associated with a user, so it requires the "android.permission.MANAGE_USERS"
+        // permission (to avoid leaking list of apps to unauthorized callers).
+        check_get_app_uids_affected_by_sid_permissions().context(ks_err!())?;
         DB.with(|db| db.borrow_mut().get_app_uids_affected_by_sid(user_id, secure_user_id))
             .context(ks_err!("Failed to get app UIDs affected by SID"))
     }
diff --git a/keystore2/src/utils.rs b/keystore2/src/utils.rs
index 174a22ba..a3fd8824 100644
--- a/keystore2/src/utils.rs
+++ b/keystore2/src/utils.rs
@@ -129,6 +129,15 @@ pub fn check_unique_id_attestation_permissions() -> anyhow::Result<()> {
     check_android_permission("android.permission.REQUEST_UNIQUE_ID_ATTESTATION")
 }
 
+/// This function checks whether the calling app has the Android permissions needed to manage
+/// users. Only callers that can manage users are allowed to get a list of apps affected
+/// by a user's SID changing.
+/// 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_get_app_uids_affected_by_sid_permissions() -> anyhow::Result<()> {
+    check_android_permission("android.permission.MANAGE_USERS")
+}
+
 fn check_android_permission(permission: &str) -> anyhow::Result<()> {
     let permission_controller: Strong<dyn IPermissionController::IPermissionController> =
         binder::get_interface("permission")?;