diff options
| author | Philip P. Moltmann <moltmann@google.com> | 2020-08-21 18:55:49 -0700 |
|---|---|---|
| committer | Philip P. Moltmann <moltmann@google.com> | 2020-09-11 19:23:07 -0700 |
| commit | bca4796525283962636e58fa4d3768cc21551913 (patch) | |
| tree | 0680942c112e3e06f547d69b8339afbc841a4154 /core/java/android/app/AppOpsManager.java | |
| parent | 5f9b30a11a7b2f4156b60a75ce73e4561f97a70c (diff) | |
Check cross-user interactions for permissions and app-ops operations
1.
We want to be quite permissive here as without being able to check
permissions or appops nothing else works. Hence allow cross-user
interactions if any cross-user permission is granted.
2.
Also we need to prevent infinite recursion as we are checking permission
and appops inside of permission and app-op checks.
2.
Clear Binder.callingUid when checking permission inside system server
Makeing the binder call "checkPermission" usually sets
Binder.callingUid to the calling processes UID. Hence clearing the
calling UID is superflous. If the call is inside the system server
though "checkPermission" is not a binder all, it is only a method call.
Hence Binder.callingUid might still be set to the app that called the
system server. This can lead to problems as not every app can check the
same permission the system server can check.
E.g. the system server can check permission accross user boundaries,
most regular apps can't
Test: atest CtsPermissionHostTestCases CtsAppOpHostTestCases // execute the new paths for both full users and profiles
atest ManagedProfileTest#testCameraPolicy // a previous version of the patch caused a regression in this test
atest AccountManagerXUserTest // a previous version of the patch caused a regression in this test
Accessed clipboard from chrome in work profile
Fixes: 153996875
Change-Id: I2a8a84f574fbf07ab88ed991445830fa85aa4450
Diffstat (limited to 'core/java/android/app/AppOpsManager.java')
| -rw-r--r-- | core/java/android/app/AppOpsManager.java | 84 |
1 files changed, 65 insertions, 19 deletions
diff --git a/core/java/android/app/AppOpsManager.java b/core/java/android/app/AppOpsManager.java index 7087b60d75dd..8ed9b80a1fcd 100644 --- a/core/java/android/app/AppOpsManager.java +++ b/core/java/android/app/AppOpsManager.java @@ -6741,10 +6741,14 @@ public class AppOpsManager { */ @RequiresPermission(android.Manifest.permission.MANAGE_APP_OPS_MODES) public void setUidMode(int code, int uid, @Mode int mode) { + // Clear calling UID to handle calls from inside the system server. See #noteOpNoThrow + long token = Binder.clearCallingIdentity(); try { mService.setUidMode(code, uid, mode); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); + } finally { + Binder.restoreCallingIdentity(token); } } @@ -6762,11 +6766,7 @@ public class AppOpsManager { @TestApi @RequiresPermission(android.Manifest.permission.MANAGE_APP_OPS_MODES) public void setUidMode(@NonNull String appOp, int uid, @Mode int mode) { - try { - mService.setUidMode(AppOpsManager.strOpToOp(appOp), uid, mode); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } + setUidMode(AppOpsManager.strOpToOp(appOp), uid, mode); } /** @hide */ @@ -6795,10 +6795,14 @@ public class AppOpsManager { @TestApi @RequiresPermission(android.Manifest.permission.MANAGE_APP_OPS_MODES) public void setMode(int code, int uid, String packageName, @Mode int mode) { + // Clear calling UID to handle calls from inside the system server. See #noteOpNoThrow + long token = Binder.clearCallingIdentity(); try { mService.setMode(code, uid, packageName, mode); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); + } finally { + Binder.restoreCallingIdentity(token); } } @@ -6818,11 +6822,7 @@ public class AppOpsManager { @RequiresPermission(android.Manifest.permission.MANAGE_APP_OPS_MODES) public void setMode(@NonNull String op, int uid, @Nullable String packageName, @Mode int mode) { - try { - mService.setMode(strOpToOp(op), uid, packageName, mode); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } + setMode(strOpToOp(op), uid, packageName, mode); } /** @@ -7298,10 +7298,14 @@ public class AppOpsManager { * @hide */ public int unsafeCheckOpRawNoThrow(int op, int uid, @NonNull String packageName) { + // Clear calling UID to handle calls from inside the system server. See #noteOpNoThrow + long token = Binder.clearCallingIdentity(); try { return mService.checkOperationRaw(op, uid, packageName); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); + } finally { + Binder.restoreCallingIdentity(token); } } @@ -7473,8 +7477,20 @@ public class AppOpsManager { } } - int mode = mService.noteOperation(op, uid, packageName, attributionTag, - collectionMode == COLLECT_ASYNC, message, shouldCollectMessage); + int mode; + // Making the binder call "noteOperation" usually sets Binder.callingUid to the calling + // processes UID. Hence clearing the calling UID is superfluous. + // If the call is inside the system server though "noteOperation" is not a binder all, + // it is only a method call. Hence Binder.callingUid might still be set to the app that + // called the system server. This can lead to problems as not every app can see the + // same appops the system server can see. + long token = Binder.clearCallingIdentity(); + try { + mode = mService.noteOperation(op, uid, packageName, attributionTag, + collectionMode == COLLECT_ASYNC, message, shouldCollectMessage); + } finally { + Binder.restoreCallingIdentity(token); + } if (mode == MODE_ALLOWED) { if (collectionMode == COLLECT_SELF) { @@ -7637,10 +7653,17 @@ public class AppOpsManager { } } - int mode = mService.noteProxyOperation(op, proxiedUid, proxiedPackageName, - proxiedAttributionTag, myUid, mContext.getOpPackageName(), - mContext.getAttributionTag(), collectionMode == COLLECT_ASYNC, message, - shouldCollectMessage); + int mode; + // Clear calling UID to handle calls from inside the system server. See #noteOpNoThrow + long token = Binder.clearCallingIdentity(); + try { + mode = mService.noteProxyOperation(op, proxiedUid, proxiedPackageName, + proxiedAttributionTag, myUid, mContext.getOpPackageName(), + mContext.getAttributionTag(), collectionMode == COLLECT_ASYNC, message, + shouldCollectMessage); + } finally { + Binder.restoreCallingIdentity(token); + } if (mode == MODE_ALLOWED) { if (collectionMode == COLLECT_SELF) { @@ -7690,6 +7713,8 @@ public class AppOpsManager { */ @UnsupportedAppUsage public int checkOp(int op, int uid, String packageName) { + // Clear calling UID to handle calls from inside the system server. See #noteOpNoThrow + long token = Binder.clearCallingIdentity(); try { int mode = mService.checkOperation(op, uid, packageName); if (mode == MODE_ERRORED) { @@ -7698,6 +7723,8 @@ public class AppOpsManager { return mode; } catch (RemoteException e) { throw e.rethrowFromSystemServer(); + } finally { + Binder.restoreCallingIdentity(token); } } @@ -7708,11 +7735,15 @@ public class AppOpsManager { */ @UnsupportedAppUsage public int checkOpNoThrow(int op, int uid, String packageName) { + // Clear calling UID to handle calls from inside the system server. See #noteOpNoThrow + long token = Binder.clearCallingIdentity(); try { int mode = mService.checkOperation(op, uid, packageName); return mode == AppOpsManager.MODE_FOREGROUND ? AppOpsManager.MODE_ALLOWED : mode; } catch (RemoteException e) { throw e.rethrowFromSystemServer(); + } finally { + Binder.restoreCallingIdentity(token); } } @@ -7964,9 +7995,16 @@ public class AppOpsManager { } } - int mode = mService.startOperation(getClientId(), op, uid, packageName, - attributionTag, startIfModeDefault, collectionMode == COLLECT_ASYNC, message, - shouldCollectMessage); + int mode; + // Clear calling UID to handle calls from inside the system server. See #noteOpNoThrow + long token = Binder.clearCallingIdentity(); + try { + mode = mService.startOperation(getClientId(), op, uid, packageName, + attributionTag, startIfModeDefault, collectionMode == COLLECT_ASYNC, + message, shouldCollectMessage); + } finally { + Binder.restoreCallingIdentity(token); + } if (mode == MODE_ALLOWED) { if (collectionMode == COLLECT_SELF) { @@ -8029,10 +8067,14 @@ public class AppOpsManager { */ public void finishOp(int op, int uid, @NonNull String packageName, @Nullable String attributionTag) { + // Clear calling UID to handle calls from inside the system server. See #noteOpNoThrow + long token = Binder.clearCallingIdentity(); try { mService.finishOperation(getClientId(), op, uid, packageName, attributionTag); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); + } finally { + Binder.restoreCallingIdentity(token); } } @@ -8624,10 +8666,14 @@ public class AppOpsManager { // TODO: Uncomment below annotation once b/73559440 is fixed // @RequiresPermission(value=Manifest.permission.WATCH_APPOPS, conditional=true) public boolean isOperationActive(int code, int uid, String packageName) { + // Clear calling UID to handle calls from inside the system server. See #noteOpNoThrow + long token = Binder.clearCallingIdentity(); try { return mService.isOperationActive(code, uid, packageName); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); + } finally { + Binder.restoreCallingIdentity(token); } } |
