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

Commit 149f620f authored by Hani Kazmi's avatar Hani Kazmi
Browse files

[AAPM] Clean up Multi User scenarios 

1. Store AAPM state as system user rather than main user. This is to
account for main user behavior potentially changing in the future.
2. Ensure only admin users can change Advanced Protection State

Bug: 398825840
Test: atest AdvancedProtectionManagerTest AdvancedProtectionServiceTest
Test: manually testing on device
Test: adb shell cmd advanced_protection set-protection-enabled true
Test: on main and secondary users
Flag: android.security.aapm_api
Change-Id: If806626f19954433265520ee8552f6206707587f
parent 0709b098

core/java/android/security/advancedprotection/AdvancedProtectionManager.java

+1 −1
Original line number Diff line number Diff line
@@ -308,7 +308,7 @@ public final class AdvancedProtectionManager { 
    }



    /**

     * Enables or disables advanced protection on the device.

     * Enables or disables advanced protection on the device. Can only be called by an admin user.

     *

     * @param enabled {@code true} to enable advanced protection, {@code false} to disable it.

     * @hide

services/core/java/com/android/server/security/advancedprotection/AdvancedProtectionService.java

+32 −16
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import android.annotation.Nullable; 
import android.app.StatsManager;

import android.content.Context;

import android.content.SharedPreferences;

import android.content.pm.UserInfo;

import android.os.Binder;

import android.os.Environment;

import android.os.Handler;
@@ -90,6 +91,7 @@ public class AdvancedProtectionService extends IAdvancedProtectionService.Stub 
    private final Context mContext;

    private final Handler mHandler;

    private final AdvancedProtectionStore mStore;

    private final UserManagerInternal mUserManager;



    // Features living with the service - their code will be executed when state changes

    private final ArrayList<AdvancedProtectionHook> mHooks = new ArrayList<>();
@@ -106,7 +108,8 @@ public class AdvancedProtectionService extends IAdvancedProtectionService.Stub 
        super(PermissionEnforcer.fromContext(context));

        mContext = context;

        mHandler = new AdvancedProtectionHandler(FgThread.get().getLooper());

        mStore = new AdvancedProtectionStore(context);

        mStore = new AdvancedProtectionStore(mContext);

        mUserManager = LocalServices.getService(UserManagerInternal.class);

    }



    private void initFeatures(boolean enabled) {
@@ -156,12 +159,18 @@ public class AdvancedProtectionService extends IAdvancedProtectionService.Stub 


    // Only for tests

    @VisibleForTesting

    AdvancedProtectionService(@NonNull Context context, @NonNull AdvancedProtectionStore store,

            @NonNull Looper looper, @NonNull PermissionEnforcer permissionEnforcer,

            @Nullable AdvancedProtectionHook hook, @Nullable AdvancedProtectionProvider provider) {

    AdvancedProtectionService(

            @NonNull Context context,

            @NonNull AdvancedProtectionStore store,

            @NonNull UserManagerInternal userManager,

            @NonNull Looper looper,

            @NonNull PermissionEnforcer permissionEnforcer,

            @Nullable AdvancedProtectionHook hook,

            @Nullable AdvancedProtectionProvider provider) {

        super(permissionEnforcer);

        mContext = context;

        mStore = store;

        mUserManager = userManager;

        mHandler = new AdvancedProtectionHandler(looper);

        if (hook != null) {

            mHooks.add(hook);
@@ -218,8 +227,10 @@ public class AdvancedProtectionService extends IAdvancedProtectionService.Stub 
    @EnforcePermission(Manifest.permission.MANAGE_ADVANCED_PROTECTION_MODE)

    public void setAdvancedProtectionEnabled(boolean enabled) {

        setAdvancedProtectionEnabled_enforcePermission();

        final UserHandle user = Binder.getCallingUserHandle();

        final long identity = Binder.clearCallingIdentity();

        try {

            enforceAdminUser(user);

            synchronized (mCallbacks) {

                if (enabled != isAdvancedProtectionEnabledInternal()) {

                    mStore.storeAdvancedProtectionModeEnabled(enabled);
@@ -364,6 +375,13 @@ public class AdvancedProtectionService extends IAdvancedProtectionService.Stub 
        return features;

    }



    private void enforceAdminUser(UserHandle user) {

        UserInfo info = mUserManager.getUserInfo(user.getIdentifier());

        if (!info.isAdmin()) {

            throw new SecurityException("Only an admin user can manage advanced protection mode");

        }

    }



    @Override

    public void onShellCommand(FileDescriptor in, FileDescriptor out,

            FileDescriptor err, @NonNull String[] args, ShellCallback callback,
@@ -451,36 +469,34 @@ public class AdvancedProtectionService extends IAdvancedProtectionService.Stub 


    @VisibleForTesting

    static class AdvancedProtectionStore {

        private final Context mContext;

        static final int ON = 1;

        static final int OFF = 0;

        private final UserManagerInternal mUserManager;

        private final Context mContext;



        AdvancedProtectionStore(@NonNull Context context) {

            mContext = context;

            mUserManager = LocalServices.getService(UserManagerInternal.class);

        }



        void storeAdvancedProtectionModeEnabled(boolean enabled) {

            Settings.Secure.putIntForUser(mContext.getContentResolver(),

                    ADVANCED_PROTECTION_MODE, enabled ? ON : OFF,

                    mUserManager.getMainUserId());

                    UserHandle.USER_SYSTEM);

        }



        boolean retrieveAdvancedProtectionModeEnabled() {

            return Settings.Secure.getIntForUser(mContext.getContentResolver(),

                    ADVANCED_PROTECTION_MODE, OFF, mUserManager.getMainUserId()) == ON;

                    ADVANCED_PROTECTION_MODE, OFF, UserHandle.USER_SYSTEM) == ON;

        }



        void storeInt(String key, int value) {

            Settings.Secure.putIntForUser(mContext.getContentResolver(),

                    key, value,

                    mUserManager.getMainUserId());

                    UserHandle.USER_SYSTEM);

        }



        int retrieveInt(String key, int defaultValue) {

            return Settings.Secure.getIntForUser(mContext.getContentResolver(),

                    key, defaultValue, mUserManager.getMainUserId());

                    key, defaultValue, UserHandle.USER_SYSTEM);

        }

    }

services/tests/servicestests/src/com/android/server/security/advancedprotection/AdvancedProtectionServiceTest.java

+143 −111
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ import static org.mockito.Mockito.mock; 
import android.Manifest;

import android.annotation.SuppressLint;

import android.content.Context;

import android.content.pm.UserInfo;

import android.os.RemoteException;

import android.os.test.FakePermissionEnforcer;

import android.os.test.TestLooper;
@@ -36,6 +37,7 @@ import android.security.advancedprotection.IAdvancedProtectionCallback; 


import androidx.annotation.NonNull;



import com.android.server.pm.UserManagerInternal;

import com.android.server.security.advancedprotection.features.AdvancedProtectionHook;

import com.android.server.security.advancedprotection.features.AdvancedProtectionProvider;
@@ -48,26 +50,37 @@ import java.util.List; 
import java.util.Map;

import java.util.HashMap;

import java.util.concurrent.atomic.AtomicBoolean;

import org.mockito.ArgumentMatchers;

import org.mockito.Mockito;



@SuppressLint("VisibleForTests")

@RunWith(JUnit4.class)

public class AdvancedProtectionServiceTest {

    private AdvancedProtectionService mService;

    private FakePermissionEnforcer mPermissionEnforcer;

    private UserManagerInternal mUserManager;

    private Context mContext;

    private AdvancedProtectionService.AdvancedProtectionStore mStore;

    private TestLooper mLooper;

    AdvancedProtectionFeature mTestFeature2g = new AdvancedProtectionFeature(

    AdvancedProtectionFeature mTestFeature2g =

            new AdvancedProtectionFeature(

                    AdvancedProtectionManager.FEATURE_ID_DISALLOW_CELLULAR_2G);



    @Before

    public void setup() throws Settings.SettingNotFoundException {

        mContext = mock(Context.class);

        mUserManager = mock(UserManagerInternal.class);

        mLooper = new TestLooper();

        mPermissionEnforcer = new FakePermissionEnforcer();

        mPermissionEnforcer.grant(Manifest.permission.MANAGE_ADVANCED_PROTECTION_MODE);

        mPermissionEnforcer.grant(Manifest.permission.QUERY_ADVANCED_PROTECTION_MODE);

        Mockito.when(mUserManager.getUserInfo(ArgumentMatchers.anyInt()))

                .thenReturn(new UserInfo(0, "user", UserInfo.FLAG_ADMIN));

    }



    private AdvancedProtectionService createService(

            AdvancedProtectionHook hook, AdvancedProtectionProvider provider) {



        mStore = new AdvancedProtectionService.AdvancedProtectionStore(mContext) {

        AdvancedProtectionService.AdvancedProtectionStore

                store = new AdvancedProtectionService.AdvancedProtectionStore(mContext) {

            private Map<String, Integer> mStoredValues = new HashMap<>();

            private boolean mEnabled = false;
@@ -92,24 +105,38 @@ public class AdvancedProtectionServiceTest { 
            }

        };



        mLooper = new TestLooper();



        mService = new AdvancedProtectionService(mContext, mStore, mLooper.getLooper(),

                mPermissionEnforcer, null, null);

        return new AdvancedProtectionService(

                mContext,

                store,

                mUserManager,

                mLooper.getLooper(),

                mPermissionEnforcer,

                hook,

                provider);

    }



    @Test

    public void testToggleProtection() {

        mService.setAdvancedProtectionEnabled(true);

        assertTrue(mService.isAdvancedProtectionEnabled());

        AdvancedProtectionService service = createService(null, null);

        service.setAdvancedProtectionEnabled(true);

        assertTrue(service.isAdvancedProtectionEnabled());



        mService.setAdvancedProtectionEnabled(false);

        assertFalse(mService.isAdvancedProtectionEnabled());

        service.setAdvancedProtectionEnabled(false);

        assertFalse(service.isAdvancedProtectionEnabled());

    }



    @Test

    public void testDisableProtection_byDefault() {

        assertFalse(mService.isAdvancedProtectionEnabled());

        AdvancedProtectionService service = createService(null, null);

        assertFalse(service.isAdvancedProtectionEnabled());

    }



    @Test

    public void testSetProtection_nonAdminUser() {

        Mockito.when(mUserManager.getUserInfo(ArgumentMatchers.anyInt()))

                .thenReturn(new UserInfo(1, "user2", UserInfo.FLAG_FULL));

        AdvancedProtectionService service = createService(null, null);

        assertThrows(SecurityException.class, () -> service.setAdvancedProtectionEnabled(true));

    }



    @Test
@@ -134,9 +161,8 @@ public class AdvancedProtectionServiceTest { 
                    }

                };



        mService = new AdvancedProtectionService(mContext, mStore, mLooper.getLooper(),

                mPermissionEnforcer, hook, null);

        mService.setAdvancedProtectionEnabled(true);

        AdvancedProtectionService service = createService(hook, null);

        service.setAdvancedProtectionEnabled(true);

        mLooper.dispatchNext();



        assertTrue(callbackCaptor.get());
@@ -164,10 +190,8 @@ public class AdvancedProtectionServiceTest { 
                    }

                };



        mService = new AdvancedProtectionService(mContext, mStore, mLooper.getLooper(),

                mPermissionEnforcer, hook, null);



        mService.setAdvancedProtectionEnabled(true);

        AdvancedProtectionService service = createService(hook, null);

        service.setAdvancedProtectionEnabled(true);

        mLooper.dispatchNext();

        assertFalse(callbackCalledCaptor.get());

    }
@@ -194,14 +218,13 @@ public class AdvancedProtectionServiceTest { 
                    }

                };



        mService = new AdvancedProtectionService(mContext, mStore, mLooper.getLooper(),

                mPermissionEnforcer, hook, null);

        mService.setAdvancedProtectionEnabled(true);

        AdvancedProtectionService service = createService(hook, null);

        service.setAdvancedProtectionEnabled(true);

        mLooper.dispatchNext();

        assertTrue(callbackCalledCaptor.get());



        callbackCalledCaptor.set(false);

        mService.setAdvancedProtectionEnabled(true);

        service.setAdvancedProtectionEnabled(true);

        mLooper.dispatchAll();

        assertFalse(callbackCalledCaptor.get());

    }
@@ -209,21 +232,24 @@ public class AdvancedProtectionServiceTest { 
    @Test

    public void testRegisterCallback() throws RemoteException {

        AtomicBoolean callbackCaptor = new AtomicBoolean(false);

        IAdvancedProtectionCallback callback = new IAdvancedProtectionCallback.Stub() {

        IAdvancedProtectionCallback callback =

                new IAdvancedProtectionCallback.Stub() {

                    @Override

                    public void onAdvancedProtectionChanged(boolean enabled) {

                        callbackCaptor.set(enabled);

                    }

                };



        mService.setAdvancedProtectionEnabled(true);

        AdvancedProtectionService service = createService(null, null);



        service.setAdvancedProtectionEnabled(true);

        mLooper.dispatchAll();



        mService.registerAdvancedProtectionCallback(callback);

        service.registerAdvancedProtectionCallback(callback);

        mLooper.dispatchNext();

        assertTrue(callbackCaptor.get());



        mService.setAdvancedProtectionEnabled(false);

        service.setAdvancedProtectionEnabled(false);

        mLooper.dispatchNext();



        assertFalse(callbackCaptor.get());
@@ -232,20 +258,23 @@ public class AdvancedProtectionServiceTest { 
    @Test

    public void testUnregisterCallback() throws RemoteException {

        AtomicBoolean callbackCalledCaptor = new AtomicBoolean(false);

        IAdvancedProtectionCallback callback = new IAdvancedProtectionCallback.Stub() {

        IAdvancedProtectionCallback callback =

                new IAdvancedProtectionCallback.Stub() {

                    @Override

                    public void onAdvancedProtectionChanged(boolean enabled) {

                        callbackCalledCaptor.set(true);

                    }

                };



        mService.setAdvancedProtectionEnabled(true);

        mService.registerAdvancedProtectionCallback(callback);

        AdvancedProtectionService service = createService(null, null);



        service.setAdvancedProtectionEnabled(true);

        service.registerAdvancedProtectionCallback(callback);

        mLooper.dispatchAll();

        callbackCalledCaptor.set(false);



        mService.unregisterAdvancedProtectionCallback(callback);

        mService.setAdvancedProtectionEnabled(false);

        service.unregisterAdvancedProtectionCallback(callback);

        service.setAdvancedProtectionEnabled(false);



        mLooper.dispatchNext();

        assertFalse(callbackCalledCaptor.get());
@@ -253,11 +282,14 @@ public class AdvancedProtectionServiceTest { 


    @Test

    public void testGetFeatures() {

        AdvancedProtectionFeature feature1 = new AdvancedProtectionFeature(

        AdvancedProtectionFeature feature1 =

                new AdvancedProtectionFeature(

                        AdvancedProtectionManager.FEATURE_ID_DISALLOW_CELLULAR_2G);

        AdvancedProtectionFeature feature2 = new AdvancedProtectionFeature(

        AdvancedProtectionFeature feature2 =

                new AdvancedProtectionFeature(

                        AdvancedProtectionManager.FEATURE_ID_DISALLOW_INSTALL_UNKNOWN_SOURCES);

        AdvancedProtectionHook hook = new AdvancedProtectionHook(mContext, true) {

        AdvancedProtectionHook hook =

                new AdvancedProtectionHook(mContext, true) {

                    @NonNull

                    @Override

                    public AdvancedProtectionFeature getFeature() {
@@ -270,26 +302,29 @@ public class AdvancedProtectionServiceTest { 
                    }

                };



        AdvancedProtectionProvider provider = new AdvancedProtectionProvider() {

        AdvancedProtectionProvider provider =

                new AdvancedProtectionProvider() {

                    @Override

                    public List<AdvancedProtectionFeature> getFeatures(Context context) {

                        return List.of(feature2);

                    }

                };



        mService = new AdvancedProtectionService(mContext, mStore, mLooper.getLooper(),

                mPermissionEnforcer, hook, provider);

        List<AdvancedProtectionFeature> features = mService.getAdvancedProtectionFeatures();

        AdvancedProtectionService service = createService(hook, provider);

        List<AdvancedProtectionFeature> features = service.getAdvancedProtectionFeatures();

        assertThat(features, containsInAnyOrder(feature1, feature2));

    }



    @Test

    public void testGetFeatures_featureNotAvailable() {

        AdvancedProtectionFeature feature1 = new AdvancedProtectionFeature(

        AdvancedProtectionFeature feature1 =

                new AdvancedProtectionFeature(

                        AdvancedProtectionManager.FEATURE_ID_DISALLOW_CELLULAR_2G);

        AdvancedProtectionFeature feature2 = new AdvancedProtectionFeature(

        AdvancedProtectionFeature feature2 =

                new AdvancedProtectionFeature(

                        AdvancedProtectionManager.FEATURE_ID_DISALLOW_INSTALL_UNKNOWN_SOURCES);

        AdvancedProtectionHook hook = new AdvancedProtectionHook(mContext, true) {

        AdvancedProtectionHook hook =

                new AdvancedProtectionHook(mContext, true) {

                    @NonNull

                    @Override

                    public AdvancedProtectionFeature getFeature() {
@@ -302,55 +337,52 @@ public class AdvancedProtectionServiceTest { 
                    }

                };



        AdvancedProtectionProvider provider = new AdvancedProtectionProvider() {

        AdvancedProtectionProvider provider =

                new AdvancedProtectionProvider() {

                    @Override

                    public List<AdvancedProtectionFeature> getFeatures(Context context) {

                        return List.of(feature2);

                    }

                };



        mService = new AdvancedProtectionService(mContext, mStore, mLooper.getLooper(),

                mPermissionEnforcer, hook, provider);

        List<AdvancedProtectionFeature> features = mService.getAdvancedProtectionFeatures();

        AdvancedProtectionService service = createService(hook, provider);

        List<AdvancedProtectionFeature> features = service.getAdvancedProtectionFeatures();

        assertThat(features, containsInAnyOrder(feature2));

    }





    @Test

    public void testSetProtection_withoutPermission() {

        AdvancedProtectionService service = createService(null, null);

        mPermissionEnforcer.revoke(Manifest.permission.MANAGE_ADVANCED_PROTECTION_MODE);

        assertThrows(SecurityException.class, () -> mService.setAdvancedProtectionEnabled(true));

        assertThrows(SecurityException.class, () -> service.setAdvancedProtectionEnabled(true));

    }



    @Test

    public void testGetProtection_withoutPermission() {

        AdvancedProtectionService service = createService(null, null);

        mPermissionEnforcer.revoke(Manifest.permission.QUERY_ADVANCED_PROTECTION_MODE);

        assertThrows(SecurityException.class, () -> mService.isAdvancedProtectionEnabled());

    }



    @Test

    public void testUsbDataProtection_withoutPermission() {

        mPermissionEnforcer.revoke(Manifest.permission.QUERY_ADVANCED_PROTECTION_MODE);

        assertThrows(SecurityException.class, () -> mService.isUsbDataProtectionEnabled());

    }



    @Test

    public void testSetUsbDataProtection_withoutPermission() {

        mPermissionEnforcer.revoke(Manifest.permission.MANAGE_ADVANCED_PROTECTION_MODE);

        assertThrows(SecurityException.class, () -> mService.setUsbDataProtectionEnabled(true));

        assertThrows(SecurityException.class, () -> service.isAdvancedProtectionEnabled());

    }



    @Test

    public void testRegisterCallback_withoutPermission() {

        AdvancedProtectionService service = createService(null, null);

        mPermissionEnforcer.revoke(Manifest.permission.QUERY_ADVANCED_PROTECTION_MODE);

        assertThrows(SecurityException.class, () -> mService.registerAdvancedProtectionCallback(

        assertThrows(

                SecurityException.class,

                () ->

                        service.registerAdvancedProtectionCallback(

                                new IAdvancedProtectionCallback.Default()));

    }



    @Test

    public void testUnregisterCallback_withoutPermission() {

        AdvancedProtectionService service = createService(null, null);

        mPermissionEnforcer.revoke(Manifest.permission.QUERY_ADVANCED_PROTECTION_MODE);

        assertThrows(SecurityException.class, () -> mService.unregisterAdvancedProtectionCallback(

        assertThrows(

                SecurityException.class,

                () ->

                        service.unregisterAdvancedProtectionCallback(

                                new IAdvancedProtectionCallback.Default()));

    }

}