Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 667c2d0f authored by Phil Weaver's avatar Phil Weaver
Browse files

Clean up a11y bindInstantService and package check

Two orthogonal changes:
1. Making bindInstantService property internal to UserState and
putting a permission check on the setter.

Having the setter in SecurityPolicy didn't make sense to me
since the field was public anyway.

I'm removing the permission check on the getter for the system,
since the field is checked anyway in getInstalledA11yServices.
So it seems like it's possible to determine the value of this
value without the permission. It also just doesn't seem like
reading this value has any security implications.

I'm doing this as part of pulling SecurityPolicy out of AMS. It
felt silly to have a permission check that was only enforced
if the caller went out of its way to have it enforced.

2. Making list of valid packages independent of the caller.

Bug: 110715236
Fixes: 118701258
Test: A11y CTS
Change-Id: I43ad0b3f3e30eae1c2a3582db672217d957919df
parent 1b4ea3a1
Loading
Loading
Loading
Loading
+5 −10
Original line number Diff line number Diff line
@@ -457,13 +457,12 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ
        final int interrogatingPid = Binder.getCallingPid();
        callback = mSystemSupport.replaceCallbackIfNeeded(callback, resolvedWindowId, interactionId,
                interrogatingPid, interrogatingTid);
        final int callingUid = Binder.getCallingUid();
        final long identityToken = Binder.clearCallingIdentity();
        try {
            connection.getRemote().findAccessibilityNodeInfosByViewId(accessibilityNodeId,
                    viewIdResName, partialInteractiveRegion, interactionId, callback, mFetchFlags,
                    interrogatingPid, interrogatingTid, spec);
            return mSecurityPolicy.computeValidReportedPackages(callingUid,
            return mSecurityPolicy.computeValidReportedPackages(
                    connection.getPackageName(), connection.getUid());
        } catch (RemoteException re) {
            if (DEBUG) {
@@ -514,13 +513,12 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ
        final int interrogatingPid = Binder.getCallingPid();
        callback = mSystemSupport.replaceCallbackIfNeeded(callback, resolvedWindowId, interactionId,
                interrogatingPid, interrogatingTid);
        final int callingUid = Binder.getCallingUid();
        final long identityToken = Binder.clearCallingIdentity();
        try {
            connection.getRemote().findAccessibilityNodeInfosByText(accessibilityNodeId,
                    text, partialInteractiveRegion, interactionId, callback, mFetchFlags,
                    interrogatingPid, interrogatingTid, spec);
            return mSecurityPolicy.computeValidReportedPackages(callingUid,
            return mSecurityPolicy.computeValidReportedPackages(
                    connection.getPackageName(), connection.getUid());
        } catch (RemoteException re) {
            if (DEBUG) {
@@ -571,13 +569,12 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ
        final int interrogatingPid = Binder.getCallingPid();
        callback = mSystemSupport.replaceCallbackIfNeeded(callback, resolvedWindowId, interactionId,
                interrogatingPid, interrogatingTid);
        final int callingUid = Binder.getCallingUid();
        final long identityToken = Binder.clearCallingIdentity();
        try {
            connection.getRemote().findAccessibilityNodeInfoByAccessibilityId(
                    accessibilityNodeId, partialInteractiveRegion, interactionId, callback,
                    mFetchFlags | flags, interrogatingPid, interrogatingTid, spec, arguments);
            return mSecurityPolicy.computeValidReportedPackages(callingUid,
            return mSecurityPolicy.computeValidReportedPackages(
                    connection.getPackageName(), connection.getUid());
        } catch (RemoteException re) {
            if (DEBUG) {
@@ -628,13 +625,12 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ
        final int interrogatingPid = Binder.getCallingPid();
        callback = mSystemSupport.replaceCallbackIfNeeded(callback, resolvedWindowId, interactionId,
                interrogatingPid, interrogatingTid);
        final int callingUid = Binder.getCallingUid();
        final long identityToken = Binder.clearCallingIdentity();
        try {
            connection.getRemote().findFocus(accessibilityNodeId, focusType,
                    partialInteractiveRegion, interactionId, callback, mFetchFlags,
                    interrogatingPid, interrogatingTid, spec);
            return mSecurityPolicy.computeValidReportedPackages(callingUid,
            return mSecurityPolicy.computeValidReportedPackages(
                    connection.getPackageName(), connection.getUid());
        } catch (RemoteException re) {
            if (DEBUG) {
@@ -684,13 +680,12 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ
        final int interrogatingPid = Binder.getCallingPid();
        callback = mSystemSupport.replaceCallbackIfNeeded(callback, resolvedWindowId, interactionId,
                interrogatingPid, interrogatingTid);
        final int callingUid = Binder.getCallingUid();
        final long identityToken = Binder.clearCallingIdentity();
        try {
            connection.getRemote().focusSearch(accessibilityNodeId, direction,
                    partialInteractiveRegion, interactionId, callback, mFetchFlags,
                    interrogatingPid, interrogatingTid, spec);
            return mSecurityPolicy.computeValidReportedPackages(callingUid,
            return mSecurityPolicy.computeValidReportedPackages(
                    connection.getPackageName(), connection.getUid());
        } catch (RemoteException re) {
            if (DEBUG) {
+44 −33
Original line number Diff line number Diff line
@@ -348,11 +348,24 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub
    }

    boolean getBindInstantServiceAllowed(int userId) {
        return  mSecurityPolicy.getBindInstantServiceAllowed(userId);
        final UserState userState = getUserState(userId);
        if (userState == null) return false;
        return userState.getBindInstantServiceAllowed();
    }

    void setBindInstantServiceAllowed(int userId, boolean allowed) {
        mSecurityPolicy.setBindInstantServiceAllowed(userId, allowed);
        UserState userState;
        synchronized (mLock) {
            userState = getUserState(userId);
            if (userState == null) {
                if (!allowed) {
                    return;
                }
                userState = new UserState(userId);
                mUserStates.put(userId, userState);
            }
        }
        userState.setBindInstantServiceAllowed(allowed);
    }

    private void registerBroadcastReceivers() {
@@ -1316,7 +1329,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub
                | PackageManager.MATCH_DIRECT_BOOT_AWARE
                | PackageManager.MATCH_DIRECT_BOOT_UNAWARE;

        if (userState.mBindInstantServiceAllowed) {
        if (userState.getBindInstantServiceAllowed()) {
            flags |= PackageManager.MATCH_INSTANT;
        }

@@ -3158,9 +3171,15 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub
            return packageNames[0];
        }

        String[] computeValidReportedPackages(int callingUid,
                String targetPackage, int targetUid) {
            if (UserHandle.getAppId(callingUid) == Process.SYSTEM_UID) {
        /**
         * Get a list of package names an app may report, including any widget packages it owns.
         *
         * @param targetPackage The known valid target package
         * @param targetUid The uid of the target app
         * @return
         */
        String[] computeValidReportedPackages(String targetPackage, int targetUid) {
            if (UserHandle.getAppId(targetUid) == Process.SYSTEM_UID) {
                // Empty array means any package is Okay
                return EmptyArray.STRING;
            }
@@ -3184,32 +3203,6 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub
            return uidPackages;
        }

        private boolean getBindInstantServiceAllowed(int userId) {
            mContext.enforceCallingOrSelfPermission(
                    Manifest.permission.MANAGE_BIND_INSTANT_SERVICE,
                    "getBindInstantServiceAllowed");
            UserState state = mUserStates.get(userId);
            return (state != null) && state.mBindInstantServiceAllowed;
        }

        private void setBindInstantServiceAllowed(int userId, boolean allowed) {
            mContext.enforceCallingOrSelfPermission(
                    Manifest.permission.MANAGE_BIND_INSTANT_SERVICE,
                    "setBindInstantServiceAllowed");
            UserState state = mUserStates.get(userId);
            if (state == null) {
                if (!allowed) {
                    return;
                }
                state = new UserState(userId);
                mUserStates.put(userId, state);
            }
            if (state.mBindInstantServiceAllowed != allowed) {
                state.mBindInstantServiceAllowed = allowed;
                onUserStateChangedLocked(state);
            }
        }

        public void clearWindowsLocked() {
            List<WindowInfo> windows = Collections.emptyList();
            final int activeWindowId = mActiveWindowId;
@@ -3819,7 +3812,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub
        public boolean mUserMinimumUiTimeoutEnabled;
        public int mUserMinimumUiTimeout;

        public boolean mBindInstantServiceAllowed;
        private boolean mBindInstantServiceAllowed;

        public UserState(int userId) {
            mUserId = userId;
@@ -4051,6 +4044,24 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub
                    Settings.Secure.ACCESSIBILITY_SOFT_KEYBOARD_MODE, 0)
                    & SHOW_MODE_HARD_KEYBOARD_ORIGINAL_VALUE) != 0;
        }

        public boolean getBindInstantServiceAllowed() {
            synchronized (mLock) {
                return mBindInstantServiceAllowed;
            }
        }

        public void setBindInstantServiceAllowed(boolean allowed) {
            synchronized (mLock) {
                mContext.enforceCallingOrSelfPermission(
                        Manifest.permission.MANAGE_BIND_INSTANT_SERVICE,
                        "setBindInstantServiceAllowed");
                if (allowed) {
                    mBindInstantServiceAllowed = allowed;
                    onUserStateChangedLocked(this);
                }
            }
        }
    }

    private final class AccessibilityContentObserver extends ContentObserver {
+1 −1
Original line number Diff line number Diff line
@@ -92,7 +92,7 @@ class AccessibilityServiceConnection extends AbstractAccessibilityServiceConnect
        final long identity = Binder.clearCallingIdentity();
        try {
            int flags = Context.BIND_AUTO_CREATE | Context.BIND_FOREGROUND_SERVICE_WHILE_AWAKE;
            if (userState.mBindInstantServiceAllowed) {
            if (userState.getBindInstantServiceAllowed()) {
                flags |= Context.BIND_ALLOW_INSTANT;
            }
            if (mService == null && mContext.bindServiceAsUser(