From 1a847ee7cbbb9166c01277430ba67e41280aa898 Mon Sep 17 00:00:00 2001 From: Ayush Sharma Date: Fri, 13 Aug 2021 15:58:12 +0000 Subject: [PATCH] Don't ret admin info from createAdminSupportIntent As part of fixing the security vulnerability(b/192245204), do not return admin package details in the returned Intent(ACTION_SHOW_ADMIN_SUPPORT_DETAILS) from createAdminSupportIntent API. Insted introduce an API to return admin info given user and restriction, this API will be used by the Activity that handles the Intent returned by createAdminSupportIntent API. Bug: 192245204 Test: atest MixedDeviceOwnerTest#testCreateAdminSupportIntent atest MixedProfileOwnerTest#testCreateAdminSupportIntent atest MixedManagedProfileOwnerTest#testCreateAdminSupportIntent atest DevicePolicyManagerTest#testCreateAdminSupportIntent Change-Id: I3ed502a078eebac62e0b8a27f6302410c22111e4 Merged-In: I3ed502a078eebac62e0b8a27f6302410c22111e4 --- .../app/admin/DevicePolicyManager.java | 21 ++ .../app/admin/IDevicePolicyManager.aidl | 1 + .../DevicePolicyManagerService.java | 215 +++++++++++------- .../devicepolicy/DevicePolicyManagerTest.java | 5 +- 4 files changed, 154 insertions(+), 88 deletions(-) diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java index 0e04ad3768c7c..0fe80c45ad2a6 100644 --- a/core/java/android/app/admin/DevicePolicyManager.java +++ b/core/java/android/app/admin/DevicePolicyManager.java @@ -9733,6 +9733,27 @@ public class DevicePolicyManager { return null; } + /** + * @param userId The user for whom to retrieve information. + * @param restriction The restriction enforced by admin. It could be any user restriction or + * policy like {@link DevicePolicyManager#POLICY_DISABLE_CAMERA} and + * {@link DevicePolicyManager#POLICY_DISABLE_SCREEN_CAPTURE}. + * @return Details of admin and user which enforced the restriction for the userId. If + * restriction is null, profile owner for the user or device owner info is returned. + * @hide + */ + public @Nullable Bundle getEnforcingAdminAndUserDetails(int userId, + @Nullable String restriction) { + if (mService != null) { + try { + return mService.getEnforcingAdminAndUserDetails(userId, restriction); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + return null; + } + /** * Hide or unhide packages. When a package is hidden it is unavailable for use, but the data and * actual package file remain. This function can be called by a device owner, profile owner, or diff --git a/core/java/android/app/admin/IDevicePolicyManager.aidl b/core/java/android/app/admin/IDevicePolicyManager.aidl index b6c48a1c057b5..b5b3934035e06 100644 --- a/core/java/android/app/admin/IDevicePolicyManager.aidl +++ b/core/java/android/app/admin/IDevicePolicyManager.aidl @@ -251,6 +251,7 @@ interface IDevicePolicyManager { boolean isNotificationListenerServicePermitted(in String packageName, int userId); Intent createAdminSupportIntent(in String restriction); + Bundle getEnforcingAdminAndUserDetails(int userId,String restriction); boolean setApplicationHidden(in ComponentName admin, in String callerPackage, in String packageName, boolean hidden, boolean parent); boolean isApplicationHidden(in ComponentName admin, in String callerPackage, in String packageName, boolean parent); diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 193d92a3b2ff9..31b4c3294e60a 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -12694,74 +12694,21 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { // This method is called from AM with its lock held, so don't take the DPMS lock. // b/29242568 - ComponentName profileOwner = mOwners.getProfileOwnerComponent(userId); - if (profileOwner != null) { - return DevicePolicyManagerService.this - .createShowAdminSupportIntent(profileOwner, userId); - } - - final Pair deviceOwner = - mOwners.getDeviceOwnerUserIdAndComponent(); - if (deviceOwner != null && deviceOwner.first == userId) { - return DevicePolicyManagerService.this - .createShowAdminSupportIntent(deviceOwner.second, userId); - } - - // We're not specifying the device admin because there isn't one. - if (useDefaultIfNoAdmin) { - return DevicePolicyManagerService.this.createShowAdminSupportIntent(null, userId); + if (getEnforcingAdminAndUserDetailsInternal(userId, null) != null + || useDefaultIfNoAdmin) { + return DevicePolicyManagerService.this.createShowAdminSupportIntent(userId); } return null; } @Override public Intent createUserRestrictionSupportIntent(int userId, String userRestriction) { - final long ident = mInjector.binderClearCallingIdentity(); - try { - final List sources = mUserManager - .getUserRestrictionSources(userRestriction, UserHandle.of(userId)); - if (sources == null || sources.isEmpty()) { - // The restriction is not enforced. - return null; - } else if (sources.size() > 1) { - // In this case, we'll show an admin support dialog that does not - // specify the admin. - // TODO(b/128928355): if this restriction is enforced by multiple DPCs, return - // the admin for the calling user. - return DevicePolicyManagerService.this.createShowAdminSupportIntent( - null, userId); - } - final UserManager.EnforcingUser enforcingUser = sources.get(0); - final int sourceType = enforcingUser.getUserRestrictionSource(); - final int enforcingUserId = enforcingUser.getUserHandle().getIdentifier(); - if (sourceType == UserManager.RESTRICTION_SOURCE_PROFILE_OWNER) { - // Restriction was enforced by PO - final ComponentName profileOwner = mOwners.getProfileOwnerComponent( - enforcingUserId); - if (profileOwner != null) { - return DevicePolicyManagerService.this.createShowAdminSupportIntent( - profileOwner, enforcingUserId); - } - } else if (sourceType == UserManager.RESTRICTION_SOURCE_DEVICE_OWNER) { - // Restriction was enforced by DO - final Pair deviceOwner = - mOwners.getDeviceOwnerUserIdAndComponent(); - if (deviceOwner != null) { - return DevicePolicyManagerService.this.createShowAdminSupportIntent( - deviceOwner.second, deviceOwner.first); - } - } else if (sourceType == UserManager.RESTRICTION_SOURCE_SYSTEM) { - /* - * In this case, the user restriction is enforced by the system. - * So we won't show an admin support intent, even if it is also - * enforced by a profile/device owner. - */ - return null; - } - } finally { - mInjector.binderRestoreCallingIdentity(ident); + Intent intent = null; + if (getEnforcingAdminAndUserDetailsInternal(userId, userRestriction) != null) { + intent = DevicePolicyManagerService.this.createShowAdminSupportIntent(userId); + intent.putExtra(DevicePolicyManager.EXTRA_RESTRICTION, userRestriction); } - return null; + return intent; } @Override @@ -13056,53 +13003,153 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } } - private Intent createShowAdminSupportIntent(ComponentName admin, int userId) { + private Intent createShowAdminSupportIntent(int userId) { // This method is called with AMS lock held, so don't take DPMS lock final Intent intent = new Intent(Settings.ACTION_SHOW_ADMIN_SUPPORT_DETAILS); intent.putExtra(Intent.EXTRA_USER_ID, userId); - intent.putExtra(DevicePolicyManager.EXTRA_DEVICE_ADMIN, admin); intent.setFlags(FLAG_ACTIVITY_NEW_TASK); return intent; } - @Override - public Intent createAdminSupportIntent(String restriction) { - Objects.requireNonNull(restriction); - final CallerIdentity caller = getCallerIdentity(); - Intent intent = null; - if (DevicePolicyManager.POLICY_DISABLE_CAMERA.equals(restriction) || - DevicePolicyManager.POLICY_DISABLE_SCREEN_CAPTURE.equals(restriction)) { + /** + * @param restriction The restriction enforced by admin. It could be any user restriction or + * policy like {@link DevicePolicyManager#POLICY_DISABLE_CAMERA} and + * {@link DevicePolicyManager#POLICY_DISABLE_SCREEN_CAPTURE}. + */ + private Bundle getEnforcingAdminAndUserDetailsInternal(int userId, String restriction) { + Bundle result = null; + if (restriction == null) { + ComponentName profileOwner = mOwners.getProfileOwnerComponent(userId); + if (profileOwner != null) { + result = new Bundle(); + result.putInt(Intent.EXTRA_USER_ID, userId); + result.putParcelable(DevicePolicyManager.EXTRA_DEVICE_ADMIN, + profileOwner); + return result; + } + final Pair deviceOwner = + mOwners.getDeviceOwnerUserIdAndComponent(); + if (deviceOwner != null && deviceOwner.first == userId) { + result = new Bundle(); + result.putInt(Intent.EXTRA_USER_ID, userId); + result.putParcelable(DevicePolicyManager.EXTRA_DEVICE_ADMIN, + deviceOwner.second); + return result; + } + } else if (DevicePolicyManager.POLICY_DISABLE_CAMERA.equals(restriction) + || DevicePolicyManager.POLICY_DISABLE_SCREEN_CAPTURE.equals(restriction)) { synchronized (getLockObject()) { - final DevicePolicyData policy = getUserData(caller.getUserId()); + final DevicePolicyData policy = getUserData(userId); final int N = policy.mAdminList.size(); for (int i = 0; i < N; i++) { final ActiveAdmin admin = policy.mAdminList.get(i); if ((admin.disableCamera && - DevicePolicyManager.POLICY_DISABLE_CAMERA.equals(restriction)) || - (admin.disableScreenCapture && DevicePolicyManager - .POLICY_DISABLE_SCREEN_CAPTURE.equals(restriction))) { - intent = createShowAdminSupportIntent(admin.info.getComponent(), - caller.getUserId()); - break; + DevicePolicyManager.POLICY_DISABLE_CAMERA.equals(restriction)) + || (admin.disableScreenCapture && DevicePolicyManager + .POLICY_DISABLE_SCREEN_CAPTURE.equals(restriction))) { + result = new Bundle(); + result.putInt(Intent.EXTRA_USER_ID, userId); + result.putParcelable(DevicePolicyManager.EXTRA_DEVICE_ADMIN, + admin.info.getComponent()); + return result; } } // For the camera, a device owner on a different user can disable it globally, // so we need an additional check. - if (intent == null + if (result == null && DevicePolicyManager.POLICY_DISABLE_CAMERA.equals(restriction)) { final ActiveAdmin admin = getDeviceOwnerAdminLocked(); if (admin != null && admin.disableCamera) { - intent = createShowAdminSupportIntent(admin.info.getComponent(), - mOwners.getDeviceOwnerUserId()); + result = new Bundle(); + result.putInt(Intent.EXTRA_USER_ID, mOwners.getDeviceOwnerUserId()); + result.putParcelable(DevicePolicyManager.EXTRA_DEVICE_ADMIN, + admin.info.getComponent()); + return result; } } } } else { - // if valid, |restriction| can only be a user restriction - intent = mLocalService.createUserRestrictionSupportIntent(caller.getUserId(), - restriction); + long ident = mInjector.binderClearCallingIdentity(); + try { + List sources = mUserManager + .getUserRestrictionSources(restriction, UserHandle.of(userId)); + if (sources == null || sources.isEmpty()) { + // The restriction is not enforced. + return null; + } else if (sources.size() > 1) { + // In this case, we'll show an admin support dialog that does not + // specify the admin. + // TODO(b/128928355): if this restriction is enforced by multiple DPCs, return + // the admin for the calling user. + result = new Bundle(); + result.putInt(Intent.EXTRA_USER_ID, userId); + return result; + } + final UserManager.EnforcingUser enforcingUser = sources.get(0); + final int sourceType = enforcingUser.getUserRestrictionSource(); + final int enforcingUserId = enforcingUser.getUserHandle().getIdentifier(); + if (sourceType == UserManager.RESTRICTION_SOURCE_PROFILE_OWNER) { + // Restriction was enforced by PO + final ComponentName profileOwner = mOwners.getProfileOwnerComponent( + enforcingUserId); + if (profileOwner != null) { + result = new Bundle(); + result.putInt(Intent.EXTRA_USER_ID, enforcingUserId); + result.putParcelable(DevicePolicyManager.EXTRA_DEVICE_ADMIN, + profileOwner); + return result; + } + } else if (sourceType == UserManager.RESTRICTION_SOURCE_DEVICE_OWNER) { + // Restriction was enforced by DO + final Pair deviceOwner = + mOwners.getDeviceOwnerUserIdAndComponent(); + if (deviceOwner != null) { + result = new Bundle(); + result.putInt(Intent.EXTRA_USER_ID, deviceOwner.first); + result.putParcelable(DevicePolicyManager.EXTRA_DEVICE_ADMIN, + deviceOwner.second); + return result; + } + } else if (sourceType == UserManager.RESTRICTION_SOURCE_SYSTEM) { + /* + * In this case, the user restriction is enforced by the system. + * So we won't show an admin support intent, even if it is also + * enforced by a profile/device owner. + */ + return null; + } + } finally { + mInjector.binderRestoreCallingIdentity(ident); + } } - if (intent != null) { + return null; + } + + /** + * @param restriction The restriction enforced by admin. It could be any user restriction or + * policy like {@link DevicePolicyManager#POLICY_DISABLE_CAMERA} and + * {@link DevicePolicyManager#POLICY_DISABLE_SCREEN_CAPTURE}. + * @return Details of admin and user which enforced the restriction for the userId. + */ + @Override + public Bundle getEnforcingAdminAndUserDetails(int userId, String restriction) { + Preconditions.checkCallAuthorization(isSystemUid(getCallerIdentity())); + return getEnforcingAdminAndUserDetailsInternal(userId, restriction); + } + + /** + * @param restriction The restriction enforced by admin. It could be any user restriction or + * policy like {@link DevicePolicyManager#POLICY_DISABLE_CAMERA} and + * {@link DevicePolicyManager#POLICY_DISABLE_SCREEN_CAPTURE}. + */ + @Override + public Intent createAdminSupportIntent(String restriction) { + Objects.requireNonNull(restriction); + final CallerIdentity caller = getCallerIdentity(); + final int userId = caller.getUserId(); + Intent intent = null; + if (getEnforcingAdminAndUserDetailsInternal(userId, restriction) != null) { + intent = createShowAdminSupportIntent(userId); intent.putExtra(DevicePolicyManager.EXTRA_RESTRICTION, restriction); } return intent; diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java index 7b20bf0f6bc70..0178c4e8c0dff 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -2960,9 +2960,6 @@ public class DevicePolicyManagerTest extends DpmTestBase { assertThat(intent.getAction()).isEqualTo(Settings.ACTION_SHOW_ADMIN_SUPPORT_DETAILS); assertThat(intent.getIntExtra(Intent.EXTRA_USER_ID, -1)) .isEqualTo(UserHandle.getUserId(DpmMockContext.CALLER_SYSTEM_USER_UID)); - assertThat( - (ComponentName) intent.getParcelableExtra(DevicePolicyManager.EXTRA_DEVICE_ADMIN)) - .isEqualTo(admin1); assertThat(intent.getStringExtra(DevicePolicyManager.EXTRA_RESTRICTION)) .isEqualTo(UserManager.DISALLOW_ADJUST_VOLUME); @@ -2999,7 +2996,7 @@ public class DevicePolicyManagerTest extends DpmTestBase { assertThat(intent.getStringExtra(DevicePolicyManager.EXTRA_RESTRICTION)) .isEqualTo(DevicePolicyManager.POLICY_DISABLE_CAMERA); assertThat(intent.getIntExtra(Intent.EXTRA_USER_ID, -1)) - .isEqualTo(UserHandle.getUserId(DpmMockContext.CALLER_SYSTEM_USER_UID)); + .isEqualTo(UserHandle.getUserId(DpmMockContext.CALLER_UID)); // ScreenCapture should not be disabled by device owner intent = dpm.createAdminSupportIntent(DevicePolicyManager.POLICY_DISABLE_SCREEN_CAPTURE); assertThat(intent).isNull();