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

Commit e725b3e9 authored by Adam Bookatz's avatar Adam Bookatz
Browse files

UserProperties won't be cached for wrong callers 

UserManager.getUserProperties() will return different results depending
on the callingUid, since different callers have different permissions.
If a process is fetching the UserProperties on behalf of a different
caller (i.e. where the callingUid doesn't match the process), then
caching the result will be wrong, since that cached copy only applied to
that particular callingUid, which the cache doesn't take into account.

In this cl, we make the cache take into account not only the userId
being queried, but also the callingUid that is querying it.

We also switch the old unusued mechanism to the new @CachedProperty
static caching mechanism.

Bug: 369198539
Bug: 367997317
Flag: android.multiuser.cache_user_properties_correctly_read_only
Test: Manual confirmation that callingUids are properly cached too
Test: Made sure we have a good hit frequency by
      enabling PropertyInvalidatedCache DEBUG and monitoring
      adb logcat  | grep PropertyInvalidatedCache | grep user_properties
Change-Id: I5a6f136f68afe84c2406e3d2c20c222eff8f381f
parent b13bf71c

core/java/android/os/UserManager.java

+50 −31
Original line number Diff line number Diff line
@@ -66,6 +66,7 @@ import android.provider.Settings; 
import android.util.AndroidException;

import android.util.ArraySet;

import android.util.Log;

import android.util.Pair;

import android.view.WindowManager.LayoutParams;



import com.android.internal.R;
@@ -3908,11 +3909,57 @@ public class UserManager { 
            android.Manifest.permission.INTERACT_ACROSS_USERS}, conditional = true)

    public @NonNull UserProperties getUserProperties(@NonNull UserHandle userHandle) {

        final int userId = userHandle.getIdentifier();



        if (userId < 0 && android.multiuser.Flags.fixGetUserPropertyCache()) {

            // Avoid calling into system server for invalid user ids.

        if (android.multiuser.Flags.fixGetUserPropertyCache() && userId < 0) {

            throw new IllegalArgumentException("Cannot access properties for user " + userId);

        }

        return mUserPropertiesCache.query(userId);



        if (!android.multiuser.Flags.cacheUserPropertiesCorrectlyReadOnly() || userId < 0) {

            // This is the historical code path, when all flags are false.

            try {

                return mService.getUserPropertiesCopy(userId);

            } catch (RemoteException re) {

                throw re.rethrowFromSystemServer();

            }

        }



        final int callingUid = Binder.getCallingUid();

        final int processUid = Process.myUid();

        if (Build.isDebuggable() && callingUid != processUid) {

            Log.w(TAG, "Uid " + processUid + " is fetching a copy of UserProperties on"

                            + " behalf of callingUid " + callingUid + ". Possibly"

                            + " it should carefully first clearCallingIdentity or perhaps use"

                            + " UserManagerInternal.getUserProperties() instead?",

                    new Throwable());

        }



        return getUserPropertiesFromQuery(new QueryUserId(userId));

    }



    /** @hide */

    public static final void invalidateUserPropertiesCache() {

        UserManagerCache.invalidateUserPropertiesFromQuery();

    }



    /**

     * Cachable version of {@link #getUserProperties(UserHandle)}, caching the UserProperties

     * corresponding to the given QueryUserId. The cached copy depends on both the queried userId as

     * well as the Binder caller querying it, since the result depends on the caller's permissions.

     */

    @CachedProperty()

    private @NonNull UserProperties getUserPropertiesFromQuery(QueryUserId query) {

        return ((UserManagerCache) mIpcDataCache).getUserPropertiesFromQuery(

                (QueryUserId q) -> mService.getUserPropertiesCopy(q.getUserId()), query);

    }



    /** Class keeping track of a userId, as well as the callingUid that is asking about it. */

    /** @hide */

    static final class QueryUserId extends Pair<Integer, Integer> {

        public QueryUserId(@UserIdInt int userId) {

            super(Binder.getCallingUid(), userId);

        }

        public @UserIdInt int getUserId() { return second; }

    }



    /**
@@ -6656,34 +6703,6 @@ public class UserManager { 
        PropertyInvalidatedCache.invalidateCache(CACHE_KEY_STATIC_USER_PROPERTIES);

    }



    /* Cache key for UserProperties object. */

    private static final String CACHE_KEY_USER_PROPERTIES =

        PropertyInvalidatedCache.createPropertyName(

            PropertyInvalidatedCache.MODULE_SYSTEM, "user_properties");



    // TODO: It would be better to somehow have this as static, so that it can work cross-context.

    private final PropertyInvalidatedCache<Integer, UserProperties> mUserPropertiesCache =

            new PropertyInvalidatedCache<Integer, UserProperties>(16, CACHE_KEY_USER_PROPERTIES) {

                @Override

                public UserProperties recompute(Integer userId) {

                    try {

                        // If the userId doesn't exist, this will throw rather than cache garbage.

                        return mService.getUserPropertiesCopy(userId);

                    } catch (RemoteException re) {

                        throw re.rethrowFromSystemServer();

                    }

                }

                @Override

                public boolean bypass(Integer query) {

                    return query < 0;

                }

            };



    /** @hide */

    public static final void invalidateUserPropertiesCache() {

        PropertyInvalidatedCache.invalidateCache(CACHE_KEY_USER_PROPERTIES);

    }



    /**

     * @hide

     * User that enforces a restriction.

services/core/java/com/android/server/pm/UserManagerService.java

+17 −13
Original line number Diff line number Diff line
@@ -1101,23 +1101,14 @@ public class UserManagerService extends IUserManager.Stub { 
        if (android.multiuser.Flags.cachesNotInvalidatedAtStartReadOnly()) {

            UserManager.invalidateIsUserUnlockedCache();

            UserManager.invalidateQuietModeEnabledCache();

            if (android.multiuser.Flags.cacheUserPropertiesCorrectlyReadOnly()) {

                UserManager.invalidateStaticUserProperties();

                UserManager.invalidateUserPropertiesCache();

            }

            UserManager.invalidateCacheOnUserListChange();

        }

    }



    private boolean doesDeviceHardwareSupportPrivateSpace() {

        return !mPm.hasSystemFeature(FEATURE_EMBEDDED, 0)

                && !mPm.hasSystemFeature(FEATURE_WATCH, 0)

                && !mPm.hasSystemFeature(FEATURE_LEANBACK, 0)

                && !mPm.hasSystemFeature(FEATURE_AUTOMOTIVE, 0);

    }



    private static boolean isAutoLockForPrivateSpaceEnabled() {

        return android.os.Flags.allowPrivateProfile()

                && Flags.supportAutolockForPrivateSpace()

                && android.multiuser.Flags.enablePrivateSpaceFeatures();

    }



    void systemReady() {

        mAppOpsService = IAppOpsService.Stub.asInterface(

                ServiceManager.getService(Context.APP_OPS_SERVICE));
@@ -1162,6 +1153,19 @@ public class UserManagerService extends IUserManager.Stub { 
        }

    }



    private boolean doesDeviceHardwareSupportPrivateSpace() {

        return !mPm.hasSystemFeature(FEATURE_EMBEDDED, 0)

                && !mPm.hasSystemFeature(FEATURE_WATCH, 0)

                && !mPm.hasSystemFeature(FEATURE_LEANBACK, 0)

                && !mPm.hasSystemFeature(FEATURE_AUTOMOTIVE, 0);

    }



    private static boolean isAutoLockForPrivateSpaceEnabled() {

        return android.os.Flags.allowPrivateProfile()

                && Flags.supportAutolockForPrivateSpace()

                && android.multiuser.Flags.enablePrivateSpaceFeatures();

    }



    private boolean isAutoLockingPrivateSpaceOnRestartsEnabled() {

        return android.os.Flags.allowPrivateProfile()

                && android.multiuser.Flags.enablePrivateSpaceAutolockOnRestarts()