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

Commit 792a0aa6 authored by Winson's avatar Winson
Browse files

Only throw SecurityException for OverlayManager callers targeting R

To maintain existing API behavior, if a legacy permission failure or
actor enforcement failure occurs for an app not targeting R,
coerce it into an IllegalStateException, which existed in the source
prior to R.

For apps targeting R, throw exceptions as they are.

Bug: 147340954

Test: atest com.android.server.om

Change-Id: Ib27ddf1db89782458c315cae2cc98a78efd9fe86
parent 0b56225e
Loading
Loading
Loading
Loading
+2 −2
Original line number Diff line number Diff line
@@ -1976,8 +1976,8 @@ package android.content.om {
  public class OverlayManager {
    method @Nullable public android.content.om.OverlayInfo getOverlayInfo(@NonNull String, @NonNull android.os.UserHandle);
    method @NonNull @RequiresPermission(anyOf={"android.permission.INTERACT_ACROSS_USERS", "android.permission.INTERACT_ACROSS_USERS_FULL"}) public java.util.List<android.content.om.OverlayInfo> getOverlayInfosForTarget(@NonNull String, @NonNull android.os.UserHandle);
    method @RequiresPermission(anyOf={"android.permission.INTERACT_ACROSS_USERS", "android.permission.INTERACT_ACROSS_USERS_FULL"}) public void setEnabled(@NonNull String, boolean, @NonNull android.os.UserHandle);
    method @RequiresPermission(anyOf={"android.permission.INTERACT_ACROSS_USERS", "android.permission.INTERACT_ACROSS_USERS_FULL"}) public void setEnabledExclusiveInCategory(@NonNull String, @NonNull android.os.UserHandle);
    method @RequiresPermission(anyOf={"android.permission.INTERACT_ACROSS_USERS", "android.permission.INTERACT_ACROSS_USERS_FULL"}) public void setEnabled(@NonNull String, boolean, @NonNull android.os.UserHandle) throws java.lang.IllegalStateException, java.lang.SecurityException;
    method @RequiresPermission(anyOf={"android.permission.INTERACT_ACROSS_USERS", "android.permission.INTERACT_ACROSS_USERS_FULL"}) public void setEnabledExclusiveInCategory(@NonNull String, @NonNull android.os.UserHandle) throws java.lang.IllegalStateException, java.lang.SecurityException;
  }
}
+45 −3
Original line number Diff line number Diff line
@@ -22,7 +22,11 @@ import android.annotation.RequiresPermission;
import android.annotation.SystemApi;
import android.annotation.SystemService;
import android.annotation.TestApi;
import android.compat.Compatibility;
import android.compat.annotation.ChangeId;
import android.compat.annotation.EnabledAfter;
import android.content.Context;
import android.os.Build;
import android.os.RemoteException;
import android.os.ServiceManager;
import android.os.UserHandle;
@@ -40,6 +44,10 @@ public class OverlayManager {
    private final IOverlayManager mService;
    private final Context mContext;

    @ChangeId
    @EnabledAfter(targetSdkVersion = Build.VERSION_CODES.Q)
    private static final long THROW_SECURITY_EXCEPTIONS = 147340954;

    /**
     * Creates a new instance.
     *
@@ -69,6 +77,9 @@ public class OverlayManager {
     * @param packageName the name of the overlay package to enable.
     * @param user The user for which to change the overlay.
     *
     * @throws SecurityException when caller is not allowed to enable {@param packageName}
     * @throws IllegalStateException when enabling fails otherwise
     *
     * @hide
     */
    @SystemApi
@@ -77,11 +88,13 @@ public class OverlayManager {
            "android.permission.INTERACT_ACROSS_USERS_FULL"
    })
    public void setEnabledExclusiveInCategory(@NonNull final String packageName,
            @NonNull UserHandle user) {
            @NonNull UserHandle user) throws SecurityException, IllegalStateException {
        try {
            if (!mService.setEnabledExclusiveInCategory(packageName, user.getIdentifier())) {
                throw new IllegalStateException("setEnabledExclusiveInCategory failed");
            }
        } catch (SecurityException e) {
            rethrowSecurityException(e);
        } catch (RemoteException e) {
            throw e.rethrowFromSystemServer();
        }
@@ -97,6 +110,9 @@ public class OverlayManager {
     * @param enable {@code false} if the overlay should be turned off.
     * @param user The user for which to change the overlay.
     *
     * @throws SecurityException when caller is not allowed to enable/disable {@param packageName}
     * @throws IllegalStateException when enabling/disabling fails otherwise
     *
     * @hide
     */
    @SystemApi
@@ -105,15 +121,16 @@ public class OverlayManager {
            "android.permission.INTERACT_ACROSS_USERS_FULL"
    })
    public void setEnabled(@NonNull final String packageName, final boolean enable,
            @NonNull UserHandle user) {
            @NonNull UserHandle user) throws SecurityException, IllegalStateException {
        try {
            if (!mService.setEnabled(packageName, enable, user.getIdentifier())) {
                throw new IllegalStateException("setEnabled failed");
            }
        } catch (SecurityException e) {
            rethrowSecurityException(e);
        } catch (RemoteException e) {
            throw e.rethrowFromSystemServer();
        }
        return;
    }

    /**
@@ -187,4 +204,29 @@ public class OverlayManager {
            throw e.rethrowFromSystemServer();
        }
    }

    /**
     * Starting on R, actor enforcement and app visibility changes introduce additional failure
     * cases, but the SecurityException thrown with these checks is unexpected for existing
     * consumers of the API.
     *
     * The only prior case it would be thrown is with a permission failure, but the calling
     * application would be able to verify that themselves, and so they may choose to ignore
     * catching SecurityException when calling these APIs.
     *
     * For R, this no longer holds true, and SecurityExceptions can be thrown for any number of
     * reasons, none of which are exposed to the caller. So for consumers targeting below R,
     * transform these SecurityExceptions into IllegalStateExceptions, which are a little more
     * expected to be thrown by the setEnabled APIs.
     *
     * This will mask the prior permission exception if it applies, but it's assumed that apps
     * wouldn't call the APIs without the permission on prior versions, and so it's safe to ignore.
     */
    private void rethrowSecurityException(SecurityException e) {
        if (!Compatibility.isChangeEnabled(THROW_SECURITY_EXCEPTIONS)) {
            throw new IllegalStateException(e);
        } else {
            throw e;
        }
    }
}
+1 −1
Original line number Diff line number Diff line
@@ -4368,7 +4368,7 @@ public class SettingsProvider extends ContentProvider {
                        try {
                            overlayManager.setEnabledExclusiveInCategory(
                                    NAV_BAR_MODE_2BUTTON_OVERLAY, UserHandle.USER_CURRENT);
                        } catch (RemoteException e) {
                        } catch (SecurityException | IllegalStateException | RemoteException e) {
                            throw new IllegalStateException(
                                    "Failed to set nav bar interaction mode overlay");
                        }
+1 −1
Original line number Diff line number Diff line
@@ -387,7 +387,7 @@ public class NavigationModeController implements Dumpable {
                    Log.d(TAG, "setModeOverlay: overlayPackage=" + overlayPkg
                            + " userId=" + userId);
                }
            } catch (RemoteException e) {
            } catch (SecurityException | IllegalStateException | RemoteException e) {
                Log.e(TAG, "Failed to enable overlay " + overlayPkg + " for user " + userId);
            }
        });
+1 −1
Original line number Diff line number Diff line
@@ -178,7 +178,7 @@ class ThemeOverlayManager {
                } else {
                    mOverlayManager.setEnabled(pkg, false, userHandle);
                }
            } catch (IllegalStateException e) {
            } catch (SecurityException | IllegalStateException e) {
                Log.e(TAG,
                        String.format("setEnabled failed: %s %s %b", pkg, userHandle, enabled), e);
            }
Loading