From b07d98eaac6bceaf843c343286ca8d88db14e682 Mon Sep 17 00:00:00 2001 From: Jackal Guo Date: Wed, 14 Aug 2019 15:23:52 +0800 Subject: [PATCH] Return false instead of throwing exception The method isCalledForCurrentedUserLocked() is used to check if callers have privilege to perform some actions. However, calling A11ySecurityPolict#resolveCallingUserIdEnforcingPermissionsLocked may get SecurityException when calls from different users without permissions directly. Checking the calling UID and user without throwing SecurityException for active a11y service instead of the original one. Bug: 62958230 Test: a11y CTS & unit tests Change-Id: I0062612bc73d507db21e151dccaf6609d9e0eaf4 --- ...bstractAccessibilityServiceConnection.java | 32 +++++++++---------- .../AccessibilitySecurityPolicy.java | 2 +- .../AccessibilityServiceConnection.java | 26 +++++++++++---- .../accessibility/UiAutomationManager.java | 2 +- ...actAccessibilityServiceConnectionTest.java | 2 +- 5 files changed, 39 insertions(+), 25 deletions(-) diff --git a/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java b/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java index 135615708037c..5e9c08b4d4809 100644 --- a/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java +++ b/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java @@ -348,13 +348,13 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ } } - protected abstract boolean isCalledForCurrentUserLocked(); + protected abstract boolean hasRightsToCurrentUserLocked(); @Override public List getWindows() { ensureWindowsAvailableTimed(); synchronized (mLock) { - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return null; } final boolean permissionGranted = @@ -387,7 +387,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ public AccessibilityWindowInfo getWindow(int windowId) { ensureWindowsAvailableTimed(); synchronized (mLock) { - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return null; } final boolean permissionGranted = @@ -420,7 +420,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ MagnificationSpec spec; synchronized (mLock) { mUsesAccessibilityCache = true; - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return null; } resolvedWindowId = resolveAccessibilityWindowIdLocked(accessibilityWindowId); @@ -481,7 +481,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ MagnificationSpec spec; synchronized (mLock) { mUsesAccessibilityCache = true; - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return null; } resolvedWindowId = resolveAccessibilityWindowIdLocked(accessibilityWindowId); @@ -542,7 +542,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ MagnificationSpec spec; synchronized (mLock) { mUsesAccessibilityCache = true; - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return null; } resolvedWindowId = resolveAccessibilityWindowIdLocked(accessibilityWindowId); @@ -602,7 +602,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ Region partialInteractiveRegion = Region.obtain(); MagnificationSpec spec; synchronized (mLock) { - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return null; } resolvedWindowId = resolveAccessibilityWindowIdForFindFocusLocked( @@ -663,7 +663,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ Region partialInteractiveRegion = Region.obtain(); MagnificationSpec spec; synchronized (mLock) { - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return null; } resolvedWindowId = resolveAccessibilityWindowIdLocked(accessibilityWindowId); @@ -728,7 +728,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ throws RemoteException { final int resolvedWindowId; synchronized (mLock) { - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return false; } resolvedWindowId = resolveAccessibilityWindowIdLocked(accessibilityWindowId); @@ -748,7 +748,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ @Override public boolean performGlobalAction(int action) { synchronized (mLock) { - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return false; } } @@ -771,7 +771,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ @Override public float getMagnificationScale(int displayId) { synchronized (mLock) { - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return 1.0f; } } @@ -787,7 +787,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ public Region getMagnificationRegion(int displayId) { synchronized (mLock) { final Region region = Region.obtain(); - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return region; } MagnificationController magnificationController = @@ -810,7 +810,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ @Override public float getMagnificationCenterX(int displayId) { synchronized (mLock) { - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return 0.0f; } MagnificationController magnificationController = @@ -832,7 +832,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ @Override public float getMagnificationCenterY(int displayId) { synchronized (mLock) { - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return 0.0f; } MagnificationController magnificationController = @@ -864,7 +864,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ @Override public boolean resetMagnification(int displayId, boolean animate) { synchronized (mLock) { - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return false; } if (!mSecurityPolicy.canControlMagnification(this)) { @@ -886,7 +886,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ public boolean setMagnificationScaleAndCenter(int displayId, float scale, float centerX, float centerY, boolean animate) { synchronized (mLock) { - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return false; } if (!mSecurityPolicy.canControlMagnification(this)) { diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilitySecurityPolicy.java b/services/accessibility/java/com/android/server/accessibility/AccessibilitySecurityPolicy.java index 315d6fa287f2a..b88b24e21c2a2 100644 --- a/services/accessibility/java/com/android/server/accessibility/AccessibilitySecurityPolicy.java +++ b/services/accessibility/java/com/android/server/accessibility/AccessibilitySecurityPolicy.java @@ -468,7 +468,7 @@ public class AccessibilitySecurityPolicy { } } - private boolean hasPermission(String permission) { + boolean hasPermission(String permission) { return mContext.checkCallingPermission(permission) == PackageManager.PERMISSION_GRANTED; } diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityServiceConnection.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityServiceConnection.java index 02f7821bd0d51..d7f61e5371d5f 100644 --- a/services/accessibility/java/com/android/server/accessibility/AccessibilityServiceConnection.java +++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityServiceConnection.java @@ -18,6 +18,7 @@ package com.android.server.accessibility; import static com.android.internal.util.function.pooled.PooledLambda.obtainMessage; +import android.Manifest; import android.accessibilityservice.AccessibilityServiceInfo; import android.accessibilityservice.IAccessibilityServiceClient; import android.content.ComponentName; @@ -27,6 +28,7 @@ import android.content.pm.ParceledListSlice; import android.os.Binder; import android.os.Handler; import android.os.IBinder; +import android.os.Process; import android.os.RemoteException; import android.os.UserHandle; import android.provider.Settings; @@ -211,19 +213,31 @@ class AccessibilityServiceConnection extends AbstractAccessibilityServiceConnect } @Override - protected boolean isCalledForCurrentUserLocked() { + protected boolean hasRightsToCurrentUserLocked() { // We treat calls from a profile as if made by its parent as profiles // share the accessibility state of the parent. The call below // performs the current profile parent resolution. - final int resolvedUserId = mSecurityPolicy - .resolveCallingUserIdEnforcingPermissionsLocked(UserHandle.USER_CURRENT); - return resolvedUserId == mSystemSupport.getCurrentUserIdLocked(); + final int callingUid = Binder.getCallingUid(); + if (callingUid == Process.ROOT_UID + || callingUid == Process.SYSTEM_UID + || callingUid == Process.SHELL_UID) { + return true; + } + if (mSecurityPolicy.resolveProfileParentLocked(UserHandle.getUserId(callingUid)) + == mSystemSupport.getCurrentUserIdLocked()) { + return true; + } + if (mSecurityPolicy.hasPermission(Manifest.permission.INTERACT_ACROSS_USERS) + || mSecurityPolicy.hasPermission(Manifest.permission.INTERACT_ACROSS_USERS_FULL)) { + return true; + } + return false; } @Override public boolean setSoftKeyboardShowMode(int showMode) { synchronized (mLock) { - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return false; } final UserState userState = mUserStateWeakReference.get(); @@ -241,7 +255,7 @@ class AccessibilityServiceConnection extends AbstractAccessibilityServiceConnect @Override public boolean isAccessibilityButtonAvailable() { synchronized (mLock) { - if (!isCalledForCurrentUserLocked()) { + if (!hasRightsToCurrentUserLocked()) { return false; } UserState userState = mUserStateWeakReference.get(); diff --git a/services/accessibility/java/com/android/server/accessibility/UiAutomationManager.java b/services/accessibility/java/com/android/server/accessibility/UiAutomationManager.java index 2698b72fdb362..b33897ed920ef 100644 --- a/services/accessibility/java/com/android/server/accessibility/UiAutomationManager.java +++ b/services/accessibility/java/com/android/server/accessibility/UiAutomationManager.java @@ -263,7 +263,7 @@ class UiAutomationManager { } @Override - protected boolean isCalledForCurrentUserLocked() { + protected boolean hasRightsToCurrentUserLocked() { // Allow UiAutomation to work for any user return true; } diff --git a/services/tests/servicestests/src/com/android/server/accessibility/AbstractAccessibilityServiceConnectionTest.java b/services/tests/servicestests/src/com/android/server/accessibility/AbstractAccessibilityServiceConnectionTest.java index 1084d625f8a3c..939aafa813e6b 100644 --- a/services/tests/servicestests/src/com/android/server/accessibility/AbstractAccessibilityServiceConnectionTest.java +++ b/services/tests/servicestests/src/com/android/server/accessibility/AbstractAccessibilityServiceConnectionTest.java @@ -752,7 +752,7 @@ public class AbstractAccessibilityServiceConnectionTest { } @Override - protected boolean isCalledForCurrentUserLocked() { + protected boolean hasRightsToCurrentUserLocked() { return mResolvedUserId == mSystemSupport.getCurrentUserIdLocked(); }