diff options
3 files changed, 176 insertions, 128 deletions
diff --git a/core/java/android/security/advancedprotection/AdvancedProtectionManager.java b/core/java/android/security/advancedprotection/AdvancedProtectionManager.java index cb2b13d1e120..ecc5fd468ee8 100644 --- a/core/java/android/security/advancedprotection/AdvancedProtectionManager.java +++ b/core/java/android/security/advancedprotection/AdvancedProtectionManager.java @@ -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 diff --git a/services/core/java/com/android/server/security/advancedprotection/AdvancedProtectionService.java b/services/core/java/com/android/server/security/advancedprotection/AdvancedProtectionService.java index 3ece07c84080..0735770f9c3c 100644 --- a/services/core/java/com/android/server/security/advancedprotection/AdvancedProtectionService.java +++ b/services/core/java/com/android/server/security/advancedprotection/AdvancedProtectionService.java @@ -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; @@ -73,7 +74,7 @@ import java.util.List; import java.util.Set; /** @hide */ -public class AdvancedProtectionService extends IAdvancedProtectionService.Stub { +public class AdvancedProtectionService extends IAdvancedProtectionService.Stub { private static final String TAG = "AdvancedProtectionService"; private static final int MODE_CHANGED = 0; private static final int CALLBACK_ADDED = 1; @@ -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); } } @@ -492,12 +508,12 @@ public class AdvancedProtectionService extends IAdvancedProtectionService.Stub @Override public void handleMessage(@NonNull Message msg) { switch (msg.what) { - //arg1 == enabled + // arg1 == enabled case MODE_CHANGED: handleAllCallbacks(msg.arg1 == 1); break; - //arg1 == enabled - //obj == callback + // arg1 == enabled + // obj == callback case CALLBACK_ADDED: handleSingleCallback(msg.arg1 == 1, (IAdvancedProtectionCallback) msg.obj); break; @@ -546,7 +562,7 @@ public class AdvancedProtectionService extends IAdvancedProtectionService.Stub private final class DeathRecipient implements IBinder.DeathRecipient { private final IBinder mBinder; - DeathRecipient(IBinder binder) { + DeathRecipient(IBinder binder) { mBinder = binder; } diff --git a/services/tests/servicestests/src/com/android/server/security/advancedprotection/AdvancedProtectionServiceTest.java b/services/tests/servicestests/src/com/android/server/security/advancedprotection/AdvancedProtectionServiceTest.java index d6b3fecb487c..ca71ed6a4130 100644 --- a/services/tests/servicestests/src/com/android/server/security/advancedprotection/AdvancedProtectionServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/security/advancedprotection/AdvancedProtectionServiceTest.java @@ -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( - AdvancedProtectionManager.FEATURE_ID_DISALLOW_CELLULAR_2G); + 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() { - @Override - public void onAdvancedProtectionChanged(boolean enabled) { - callbackCaptor.set(enabled); - } - }; + IAdvancedProtectionCallback callback = + new IAdvancedProtectionCallback.Stub() { + @Override + public void onAdvancedProtectionChanged(boolean enabled) { + callbackCaptor.set(enabled); + } + }; + + AdvancedProtectionService service = createService(null, null); - mService.setAdvancedProtectionEnabled(true); + 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() { - @Override - public void onAdvancedProtectionChanged(boolean enabled) { - callbackCalledCaptor.set(true); - } - }; + IAdvancedProtectionCallback callback = + new IAdvancedProtectionCallback.Stub() { + @Override + public void onAdvancedProtectionChanged(boolean enabled) { + callbackCalledCaptor.set(true); + } + }; + + AdvancedProtectionService service = createService(null, null); - mService.setAdvancedProtectionEnabled(true); - mService.registerAdvancedProtectionCallback(callback); + 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,104 +282,107 @@ public class AdvancedProtectionServiceTest { @Test public void testGetFeatures() { - AdvancedProtectionFeature feature1 = new AdvancedProtectionFeature( - AdvancedProtectionManager.FEATURE_ID_DISALLOW_CELLULAR_2G); - AdvancedProtectionFeature feature2 = new AdvancedProtectionFeature( - AdvancedProtectionManager.FEATURE_ID_DISALLOW_INSTALL_UNKNOWN_SOURCES); - AdvancedProtectionHook hook = new AdvancedProtectionHook(mContext, true) { - @NonNull - @Override - public AdvancedProtectionFeature getFeature() { - return feature1; - } + AdvancedProtectionFeature feature1 = + new AdvancedProtectionFeature( + AdvancedProtectionManager.FEATURE_ID_DISALLOW_CELLULAR_2G); + AdvancedProtectionFeature feature2 = + new AdvancedProtectionFeature( + AdvancedProtectionManager.FEATURE_ID_DISALLOW_INSTALL_UNKNOWN_SOURCES); + AdvancedProtectionHook hook = + new AdvancedProtectionHook(mContext, true) { + @NonNull + @Override + public AdvancedProtectionFeature getFeature() { + return feature1; + } - @Override - public boolean isAvailable() { - return true; - } - }; + @Override + public boolean isAvailable() { + return true; + } + }; - AdvancedProtectionProvider provider = new AdvancedProtectionProvider() { - @Override - public List<AdvancedProtectionFeature> getFeatures(Context context) { - return List.of(feature2); - } - }; + 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( - AdvancedProtectionManager.FEATURE_ID_DISALLOW_CELLULAR_2G); - AdvancedProtectionFeature feature2 = new AdvancedProtectionFeature( - AdvancedProtectionManager.FEATURE_ID_DISALLOW_INSTALL_UNKNOWN_SOURCES); - AdvancedProtectionHook hook = new AdvancedProtectionHook(mContext, true) { - @NonNull - @Override - public AdvancedProtectionFeature getFeature() { - return feature1; - } + AdvancedProtectionFeature feature1 = + new AdvancedProtectionFeature( + AdvancedProtectionManager.FEATURE_ID_DISALLOW_CELLULAR_2G); + AdvancedProtectionFeature feature2 = + new AdvancedProtectionFeature( + AdvancedProtectionManager.FEATURE_ID_DISALLOW_INSTALL_UNKNOWN_SOURCES); + AdvancedProtectionHook hook = + new AdvancedProtectionHook(mContext, true) { + @NonNull + @Override + public AdvancedProtectionFeature getFeature() { + return feature1; + } - @Override - public boolean isAvailable() { - return false; - } - }; + @Override + public boolean isAvailable() { + return false; + } + }; - AdvancedProtectionProvider provider = new AdvancedProtectionProvider() { - @Override - public List<AdvancedProtectionFeature> getFeatures(Context context) { - return List.of(feature2); - } - }; + 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( - new IAdvancedProtectionCallback.Default())); + 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( - new IAdvancedProtectionCallback.Default())); + assertThrows( + SecurityException.class, + () -> + service.unregisterAdvancedProtectionCallback( + new IAdvancedProtectionCallback.Default())); } } |