From ac4ef4db6f25417398a0fb33da41730ca34f2281 Mon Sep 17 00:00:00 2001 From: Ayush Sharma Date: Mon, 23 Aug 2021 21:29:28 +0000 Subject: [PATCH] Enforce missing permission getCameraDisabled Add following checks: * Caller has cross user permission if user other than caller is queried * If componentName is specified than it belongs to the caller. Also, Merged security exceptions in getCallerIdentity, as there was a information leak. From different security exceptions it could be deduced if particular package is admin package. Bug: 193033501 Test: atest MixedProfileOwnerHostSideTransferTest#testTransferPolicies atest OrgOwnedProfileOwnerTest#testCameraDisabledOnParentIsEnforced atest ManagedProfileTest#testCameraPolicy atest MixedDeviceOwnerTest#testSetCameraDisabledLogged atest MixedProfileOwnerTest#testSetCameraDisabledLogged atest MixedManagedProfileOwnerTest#testSetCameraDisabledLogged atest com.android.server.devicepolicy.DevicePolicyManagerTest#testDaDisallowedPolicies_SecurityException atest com.android.server.devicepolicy.DevicePolicyManagerTest#testGetMacAddress Change-Id: I521ffd81f93c5c2576e9324152856b4a4933c094 Merged-In: I521ffd81f93c5c2576e9324152856b4a4933c094 (cherry picked from commit 30e223d1321794cb7720f51c3626a883e0db5880) --- .../DevicePolicyManagerService.java | 32 ++++++++----------- .../devicepolicy/DevicePolicyManagerTest.java | 10 ++++-- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index c283ef07dce6f..0e47c6e418b5f 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -1981,13 +1981,12 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { final DevicePolicyData policy = getUserData(UserHandle.getUserId(callerUid)); ActiveAdmin admin = policy.mAdminMap.get(adminComponent); - if (admin == null) { + // Throwing combined exception message for both the cases here, because from different + // security exceptions it could be deduced if particular package is admin package. + if (admin == null || admin.getUid() != callerUid) { throw new SecurityException(String.format( - "No active admin for %s", adminComponent)); - } - if (admin.getUid() != callerUid) { - throw new SecurityException(String.format( - "Admin %s is not owned by uid %d", adminComponent, callerUid)); + "Admin %s does not exist or is not owned by uid %d", adminComponent, + callerUid)); } if (callerPackage != null) { Preconditions.checkArgument(callerPackage.equals(adminComponent.getPackageName())); @@ -8153,17 +8152,16 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { */ @Override public boolean getCameraDisabled(ComponentName who, int userHandle, boolean parent) { - return getCameraDisabled(who, userHandle, /* mergeDeviceOwnerRestriction= */ true, parent); - } - - private boolean getCameraDisabled(ComponentName who, int userHandle, - boolean mergeDeviceOwnerRestriction, boolean parent) { if (!mHasFeature) { return false; } + + final CallerIdentity caller = getCallerIdentity(who); + Preconditions.checkCallAuthorization(hasFullCrossUsersPermission(caller, userHandle)); + if (parent) { Preconditions.checkCallAuthorization( - isProfileOwnerOfOrganizationOwnedDevice(getCallerIdentity().getUserId())); + isProfileOwnerOfOrganizationOwnedDevice(caller.getUserId())); } synchronized (getLockObject()) { @@ -8172,17 +8170,15 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { return (admin != null) && admin.disableCamera; } // First, see if DO has set it. If so, it's device-wide. - if (mergeDeviceOwnerRestriction) { - final ActiveAdmin deviceOwner = getDeviceOwnerAdminLocked(); - if (deviceOwner != null && deviceOwner.disableCamera) { - return true; - } + final ActiveAdmin deviceOwner = getDeviceOwnerAdminLocked(); + if (deviceOwner != null && deviceOwner.disableCamera) { + return true; } final int affectedUserId = parent ? getProfileParentId(userHandle) : userHandle; // Return the strictest policy across all participating admins. List admins = getActiveAdminsForAffectedUserLocked(affectedUserId); // Determine whether or not the device camera is disabled for any active admins. - for (ActiveAdmin admin: admins) { + for (ActiveAdmin admin : admins) { if (admin.disableCamera) { return true; } 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..66f77bd11cd96 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -101,6 +101,7 @@ import android.graphics.Color; import android.hardware.usb.UsbManager; import android.net.ConnectivityManager; import android.net.Uri; +import android.os.Build; import android.os.Build.VERSION_CODES; import android.os.Bundle; import android.os.Process; @@ -2096,9 +2097,12 @@ public class DevicePolicyManagerTest extends DpmTestBase { mContext.callerPermissions.add(permission.MANAGE_DEVICE_ADMINS); mContext.callerPermissions.add(permission.INTERACT_ACROSS_USERS_FULL); - setUpPackageManagerForAdmin(admin1, DpmMockContext.CALLER_SYSTEM_USER_UID); + setUpPackageManagerForAdmin(admin1, DpmMockContext.CALLER_SYSTEM_USER_UID, null, + Build.VERSION_CODES.Q); dpm.setActiveAdmin(admin1, /* replace =*/ false, UserHandle.USER_SYSTEM); + + mContext.binder.callingUid = DpmMockContext.CALLER_SYSTEM_USER_UID; boolean originalCameraDisabled = dpm.getCameraDisabled(admin1); assertExpectException(SecurityException.class, /* messageRegex= */ null, () -> dpm.setCameraDisabled(admin1, true)); @@ -2665,8 +2669,8 @@ public class DevicePolicyManagerTest extends DpmTestBase { setUpPackageManagerForAdmin(admin1, DpmMockContext.CALLER_SYSTEM_USER_UID); // Test 1. Caller doesn't have DO or DA. - assertExpectException(SecurityException.class, /* messageRegex= */ "No active admin", - () -> dpm.getWifiMacAddress(admin1)); + assertExpectException(SecurityException.class, /* messageRegex= */ + "does not exist or is not owned by uid", () -> dpm.getWifiMacAddress(admin1)); // DO needs to be an DA. dpm.setActiveAdmin(admin1, /* replace =*/ false);