From 421dcbaca58eb544ba35f1ca07d424e009d1369f Mon Sep 17 00:00:00 2001 From: Yan Zhu Date: Sun, 6 Jun 2021 23:36:18 -0700 Subject: [PATCH] Improve restriction for BugreportManagerService for multi-user For provisioned device - Remove the check for the current user is the same as the primary user - Check for whether the caller is device owner and the current user is affiliated with the device Bug: 185426804 Test: manual test with AAOS build with device owner setup and request bugreport from TestDPC Change-Id: I78066979fd54b480433ef9ba5144ca666c601591 --- .../app/admin/DevicePolicyManager.java | 14 ++++++- .../app/admin/IDevicePolicyManager.aidl | 3 +- .../os/BugreportManagerServiceImpl.java | 38 +++++++++++++++++-- .../DevicePolicyManagerService.java | 13 ++++++- 4 files changed, 62 insertions(+), 6 deletions(-) diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java index 52c58e1622895..b5b7dbdff1725 100644 --- a/core/java/android/app/admin/DevicePolicyManager.java +++ b/core/java/android/app/admin/DevicePolicyManager.java @@ -11871,7 +11871,19 @@ public class DevicePolicyManager { public boolean isAffiliatedUser() { throwIfParentInstance("isAffiliatedUser"); try { - return mService.isAffiliatedUser(); + return mService.isCallingUserAffiliated(); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + + /** + * @hide + * Returns whether target user is affiliated with the device. + */ + public boolean isAffiliatedUser(@UserIdInt int userId) { + try { + return mService.isAffiliatedUser(userId); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } diff --git a/core/java/android/app/admin/IDevicePolicyManager.aidl b/core/java/android/app/admin/IDevicePolicyManager.aidl index db2fc0d19b4c9..b6c48a1c057b5 100644 --- a/core/java/android/app/admin/IDevicePolicyManager.aidl +++ b/core/java/android/app/admin/IDevicePolicyManager.aidl @@ -390,7 +390,8 @@ interface IDevicePolicyManager { void setAffiliationIds(in ComponentName admin, in List ids); List getAffiliationIds(in ComponentName admin); - boolean isAffiliatedUser(); + boolean isCallingUserAffiliated(); + boolean isAffiliatedUser(int userId); void setSecurityLoggingEnabled(in ComponentName admin, String packageName, boolean enabled); boolean isSecurityLoggingEnabled(in ComponentName admin, String packageName); diff --git a/services/core/java/com/android/server/os/BugreportManagerServiceImpl.java b/services/core/java/com/android/server/os/BugreportManagerServiceImpl.java index 293c59d1831dc..346f3113f25f8 100644 --- a/services/core/java/com/android/server/os/BugreportManagerServiceImpl.java +++ b/services/core/java/com/android/server/os/BugreportManagerServiceImpl.java @@ -20,6 +20,7 @@ import android.annotation.Nullable; import android.annotation.RequiresPermission; import android.app.ActivityManager; import android.app.AppOpsManager; +import android.app.admin.DevicePolicyManager; import android.content.Context; import android.content.pm.PackageManager; import android.content.pm.UserInfo; @@ -31,6 +32,7 @@ import android.os.RemoteException; import android.os.ServiceManager; import android.os.SystemClock; import android.os.SystemProperties; +import android.os.UserHandle; import android.os.UserManager; import android.telephony.TelephonyManager; import android.util.ArraySet; @@ -81,7 +83,7 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { == BugreportParams.BUGREPORT_MODE_TELEPHONY /* checkCarrierPrivileges */); final long identity = Binder.clearCallingIdentity(); try { - ensureIsPrimaryUser(); + ensureUserCanTakeBugReport(bugreportMode); } finally { Binder.restoreCallingIdentity(identity); } @@ -166,11 +168,12 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { } /** - * Validates that the current user is the primary user. + * Validates that the current user is the primary user or when bugreport is requested remotely + * and current user is affiliated user. * * @throws IllegalArgumentException if the current user is not the primary user */ - private void ensureIsPrimaryUser() { + private void ensureUserCanTakeBugReport(int bugreportMode) { UserInfo currentUser = null; try { currentUser = ActivityManager.getService().getCurrentUser(); @@ -186,11 +189,40 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { logAndThrow("No primary user. Only primary user is allowed to take bugreports."); } if (primaryUser.id != currentUser.id) { + if (bugreportMode == BugreportParams.BUGREPORT_MODE_REMOTE + && isCurrentUserAffiliated(currentUser.id)) { + return; + } logAndThrow("Current user not primary user. Only primary user" + " is allowed to take bugreports."); } } + /** + * Returns {@code true} if the device has device owner and the current user is affiliated + * with the device owner. + */ + private boolean isCurrentUserAffiliated(int currentUserId) { + DevicePolicyManager dpm = mContext.getSystemService(DevicePolicyManager.class); + int deviceOwnerUid = dpm.getDeviceOwnerUserId(); + if (deviceOwnerUid == UserHandle.USER_NULL) { + return false; + } + + int callingUserId = UserHandle.getUserId(Binder.getCallingUid()); + + Slog.i(TAG, "callingUid: " + callingUserId + " deviceOwnerUid: " + deviceOwnerUid + + " currentUserId: " + currentUserId); + + if (callingUserId != deviceOwnerUid) { + logAndThrow("Caller is not device owner on provisioned device."); + } + if (!dpm.isAffiliatedUser(currentUserId)) { + logAndThrow("Current user is not affiliated to the device owner."); + } + return true; + } + @GuardedBy("mLock") private void startBugreportLocked(int callingUid, String callingPackage, FileDescriptor bugreportFd, FileDescriptor screenshotFd, diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 0128d350bd101..d0b7c61c1e00c 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -14313,7 +14313,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } @Override - public boolean isAffiliatedUser() { + public boolean isCallingUserAffiliated() { if (!mHasFeature) { return false; } @@ -14323,6 +14323,17 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } } + @Override + public boolean isAffiliatedUser(@UserIdInt int userId) { + if (!mHasFeature) { + return false; + } + final CallerIdentity caller = getCallerIdentity(); + Preconditions.checkCallAuthorization(hasCrossUsersPermission(caller, userId)); + + return isUserAffiliatedWithDeviceLocked(userId); + } + private boolean isUserAffiliatedWithDeviceLocked(@UserIdInt int userId) { if (!mOwners.hasDeviceOwner()) { return false;