diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java index cb2a45ddc6245..8c9e771dbd737 100644 --- a/core/java/android/app/admin/DevicePolicyManager.java +++ b/core/java/android/app/admin/DevicePolicyManager.java @@ -6845,6 +6845,10 @@ public class DevicePolicyManager { *

Enabling lockdown via {@code lockdownEnabled} argument carries the risk that any failure * of the VPN provider could break networking for all apps. This method clears any lockdown * allowlist set by {@link #setAlwaysOnVpnPackage(ComponentName, String, boolean, Set)}. + *

Starting from {@link android.os.Build.VERSION_CODES#S API 31} calling this method with + * {@code vpnPackage} set to {@code null} only removes the existing configuration if it was + * previously created by this admin. To remove VPN configuration created by the user use + * {@link UserManager#DISALLOW_CONFIG_VPN}. * * @param vpnPackage The package name for an installed VPN app on the device, or {@code null} to * remove an existing always-on VPN configuration. diff --git a/core/java/android/app/admin/DevicePolicyManagerInternal.java b/core/java/android/app/admin/DevicePolicyManagerInternal.java index 67f5c366bc141..a9bec98ce405b 100644 --- a/core/java/android/app/admin/DevicePolicyManagerInternal.java +++ b/core/java/android/app/admin/DevicePolicyManagerInternal.java @@ -265,5 +265,4 @@ public abstract class DevicePolicyManagerInternal { */ public abstract void notifyUnsafeOperationStateChanged(DevicePolicySafetyChecker checker, @OperationSafetyReason int reason, boolean isSafe); - } diff --git a/core/java/android/os/UserManager.java b/core/java/android/os/UserManager.java index 224cd84bc777a..c22224dd0da2a 100644 --- a/core/java/android/os/UserManager.java +++ b/core/java/android/os/UserManager.java @@ -575,6 +575,8 @@ public class UserManager { *

This restriction also prevents VPNs from starting. However, in Android 7.0 * ({@linkplain android.os.Build.VERSION_CODES#N API level 24}) or higher, the system does * start always-on VPNs created by the device or profile owner. + *

From Android 12 ({@linkplain android.os.Build.VERSION_CODES#S API level 31}) enforcing + * this restriction clears currently active VPN if it was configured by the user. * *

Key for user restrictions. *

Type: Boolean diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 9ff534046d118..1d27655055a00 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -22,6 +22,7 @@ import static android.Manifest.permission.REQUEST_PASSWORD_COMPLEXITY; import static android.accessibilityservice.AccessibilityServiceInfo.FEEDBACK_ALL_MASK; import static android.app.ActivityManager.LOCK_TASK_MODE_NONE; import static android.app.AppOpsManager.MODE_ALLOWED; +import static android.app.AppOpsManager.MODE_DEFAULT; import static android.app.admin.DeviceAdminReceiver.ACTION_COMPLIANCE_ACKNOWLEDGEMENT_REQUIRED; import static android.app.admin.DeviceAdminReceiver.EXTRA_TRANSFER_OWNERSHIP_ADMIN_EXTRAS_BUNDLE; import static android.app.admin.DevicePolicyManager.ACTION_CHECK_POLICY_COMPLIANCE; @@ -990,13 +991,24 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { @Override public void onUserRestrictionsChanged(int userId, Bundle newRestrictions, Bundle prevRestrictions) { - final boolean newlyDisallowed = - newRestrictions.getBoolean(UserManager.DISALLOW_SHARE_INTO_MANAGED_PROFILE); - final boolean previouslyDisallowed = - prevRestrictions.getBoolean(UserManager.DISALLOW_SHARE_INTO_MANAGED_PROFILE); - final boolean restrictionChanged = (newlyDisallowed != previouslyDisallowed); + resetCrossProfileIntentFiltersIfNeeded(userId, newRestrictions, prevRestrictions); + resetUserVpnIfNeeded(userId, newRestrictions, prevRestrictions); + } - if (restrictionChanged) { + private void resetUserVpnIfNeeded( + int userId, Bundle newRestrictions, Bundle prevRestrictions) { + final boolean newlyEnforced = + !prevRestrictions.getBoolean(UserManager.DISALLOW_CONFIG_VPN) + && newRestrictions.getBoolean(UserManager.DISALLOW_CONFIG_VPN); + if (newlyEnforced) { + mDpms.clearUserConfiguredVpns(userId); + } + } + + private void resetCrossProfileIntentFiltersIfNeeded( + int userId, Bundle newRestrictions, Bundle prevRestrictions) { + if (UserRestrictionsUtils.restrictionsChanged(prevRestrictions, newRestrictions, + UserManager.DISALLOW_SHARE_INTO_MANAGED_PROFILE)) { final int parentId = mUserManagerInternal.getProfileParentId(userId); if (parentId == userId) { return; @@ -1007,13 +1019,55 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { Slogf.i(LOG_TAG, "Resetting cross-profile intent filters on restriction " + "change"); mDpms.resetDefaultCrossProfileIntentFilters(parentId); - mContext.sendBroadcastAsUser(new Intent( - DevicePolicyManager.ACTION_DATA_SHARING_RESTRICTION_APPLIED), + mContext.sendBroadcastAsUser( + new Intent(DevicePolicyManager.ACTION_DATA_SHARING_RESTRICTION_APPLIED), UserHandle.of(userId)); } } } + private void clearUserConfiguredVpns(int userId) { + final String adminConfiguredVpnPkg; + synchronized (getLockObject()) { + final ActiveAdmin owner = getDeviceOrProfileOwnerAdminLocked(userId); + if (owner == null) { + Slogf.wtf(LOG_TAG, "Admin not found"); + return; + } + adminConfiguredVpnPkg = owner.mAlwaysOnVpnPackage; + } + + // Clear always-on configuration if it wasn't set by the admin. + if (adminConfiguredVpnPkg == null) { + mInjector.getVpnManager().setAlwaysOnVpnPackageForUser(userId, null, false, null); + } + + // Clear app authorizations to establish VPNs. When DISALLOW_CONFIG_VPN is enforced apps + // won't be able to get those authorizations unless it is configured by an admin. + final List allVpnOps = mInjector.getAppOpsManager() + .getPackagesForOps(new int[] {AppOpsManager.OP_ACTIVATE_VPN}); + if (allVpnOps == null) { + return; + } + for (AppOpsManager.PackageOps pkgOps : allVpnOps) { + if (UserHandle.getUserId(pkgOps.getUid()) != userId + || pkgOps.getPackageName().equals(adminConfiguredVpnPkg)) { + continue; + } + if (pkgOps.getOps().size() != 1) { + Slogf.wtf(LOG_TAG, "Unexpected number of ops returned"); + continue; + } + final @Mode int mode = pkgOps.getOps().get(0).getMode(); + if (mode == MODE_ALLOWED) { + Slogf.i(LOG_TAG, String.format("Revoking VPN authorization for package %s uid %d", + pkgOps.getPackageName(), pkgOps.getUid())); + mInjector.getAppOpsManager().setMode(AppOpsManager.OP_ACTIVATE_VPN, pkgOps.getUid(), + pkgOps.getPackageName(), MODE_DEFAULT); + } + } + } + private final class UserLifecycleListener implements UserManagerInternal.UserLifecycleListener { @Override @@ -6559,6 +6613,19 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { Preconditions.checkCallAuthorization(isDeviceOwner(caller) || isProfileOwner(caller)); checkCanExecuteOrThrowUnsafe(DevicePolicyManager.OPERATION_SET_ALWAYS_ON_VPN_PACKAGE); + if (vpnPackage == null) { + final String prevVpnPackage; + synchronized (getLockObject()) { + prevVpnPackage = getProfileOwnerOrDeviceOwnerLocked(caller).mAlwaysOnVpnPackage; + // If the admin is clearing VPN package but hasn't configure any VPN previously, + // ignore it so that it doesn't interfere with user-configured VPNs. + if (TextUtils.isEmpty(prevVpnPackage)) { + return true; + } + } + revokeVpnAuthorizationForPackage(prevVpnPackage, caller.getUserId()); + } + final int userId = caller.getUserId(); mInjector.binderWithCleanCallingIdentity(() -> { if (vpnPackage != null && !isPackageInstalledForUser(vpnPackage, userId)) { @@ -6581,14 +6648,14 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { userId, vpnPackage, lockdown, lockdownAllowlist)) { throw new UnsupportedOperationException(); } - DevicePolicyEventLogger - .createEvent(DevicePolicyEnums.SET_ALWAYS_ON_VPN_PACKAGE) - .setAdmin(caller.getComponentName()) - .setStrings(vpnPackage) - .setBoolean(lockdown) - .setInt(lockdownAllowlist != null ? lockdownAllowlist.size() : 0) - .write(); }); + DevicePolicyEventLogger + .createEvent(DevicePolicyEnums.SET_ALWAYS_ON_VPN_PACKAGE) + .setAdmin(caller.getComponentName()) + .setStrings(vpnPackage) + .setBoolean(lockdown) + .setInt(lockdownAllowlist != null ? lockdownAllowlist.size() : 0) + .write(); synchronized (getLockObject()) { ActiveAdmin admin = getProfileOwnerOrDeviceOwnerLocked(caller); if (!TextUtils.equals(vpnPackage, admin.mAlwaysOnVpnPackage) @@ -6601,6 +6668,23 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { return true; } + private void revokeVpnAuthorizationForPackage(String vpnPackage, int userId) { + mInjector.binderWithCleanCallingIdentity(() -> { + try { + final ApplicationInfo ai = mIPackageManager.getApplicationInfo( + vpnPackage, /* flags= */ 0, userId); + if (ai == null) { + Slogf.w(LOG_TAG, "Non-existent VPN package: " + vpnPackage); + } else { + mInjector.getAppOpsManager().setMode(AppOpsManager.OP_ACTIVATE_VPN, + ai.uid, vpnPackage, MODE_DEFAULT); + } + } catch (RemoteException e) { + Slogf.e(LOG_TAG, "Can't talk to package managed", e); + } + }); + } + @Override public String getAlwaysOnVpnPackage(ComponentName admin) throws SecurityException { Objects.requireNonNull(admin, "ComponentName is null"); @@ -8390,12 +8474,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { return who != null && who.equals(profileOwner); } - private boolean isProfileOwnerUncheckedLocked(ComponentName who, int userId) { - ensureLocked(); - final ComponentName profileOwner = mOwners.getProfileOwnerComponent(userId); - return who != null && who.equals(profileOwner); - } - /** * Returns {@code true} if the provided caller identity is of a profile owner. * @param caller identity of caller. @@ -13667,16 +13745,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { + " is not device owner"); } - private ComponentName getOwnerComponent(String packageName, int userId) { - if (isDeviceOwnerPackage(packageName, userId)) { - return mOwners.getDeviceOwnerComponent(); - } - if (isProfileOwnerPackage(packageName, userId)) { - return mOwners.getProfileOwnerComponent(userId); - } - return null; - } - /** * Return device owner or profile owner set on a given user. */ diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java index 654d9fcf9210e..fd41b9c19a2ee 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -15,6 +15,9 @@ */ package com.android.server.devicepolicy; +import static android.app.AppOpsManager.MODE_ALLOWED; +import static android.app.AppOpsManager.MODE_DEFAULT; +import static android.app.AppOpsManager.OP_ACTIVATE_VPN; import static android.app.Notification.EXTRA_TEXT; import static android.app.Notification.EXTRA_TITLE; import static android.app.admin.DevicePolicyManager.ACTION_CHECK_POLICY_COMPLIANCE; @@ -7436,6 +7439,101 @@ public class DevicePolicyManagerTest extends DpmTestBase { assertThrows(SecurityException.class, () -> dpm.setRecommendedGlobalProxy(admin1, null)); } + @Test + public void testSetAlwaysOnVpnPackage_clearsAdminVpn() throws Exception { + setDeviceOwner(); + + when(getServices().vpnManager + .setAlwaysOnVpnPackageForUser(anyInt(), any(), anyBoolean(), any())) + .thenReturn(true); + + // Set VPN package to admin package. + dpm.setAlwaysOnVpnPackage(admin1, admin1.getPackageName(), false, null); + + verify(getServices().vpnManager).setAlwaysOnVpnPackageForUser( + UserHandle.USER_SYSTEM, admin1.getPackageName(), false, null); + + // Clear VPN package. + dpm.setAlwaysOnVpnPackage(admin1, null, false, null); + + // Change should be propagated to VpnManager + verify(getServices().vpnManager).setAlwaysOnVpnPackageForUser( + UserHandle.USER_SYSTEM, null, false, null); + // The package should lose authorization to start VPN. + verify(getServices().appOpsManager).setMode(OP_ACTIVATE_VPN, + DpmMockContext.CALLER_SYSTEM_USER_UID, admin1.getPackageName(), MODE_DEFAULT); + } + + @Test + public void testSetAlwaysOnVpnPackage_doesntKillUserVpn() throws Exception { + setDeviceOwner(); + + when(getServices().vpnManager + .setAlwaysOnVpnPackageForUser(anyInt(), any(), anyBoolean(), any())) + .thenReturn(true); + + // this time it shouldn't go into VpnManager anymore. + dpm.setAlwaysOnVpnPackage(admin1, null, false, null); + + verifyNoMoreInteractions(getServices().vpnManager); + verifyNoMoreInteractions(getServices().appOpsManager); + } + + @Test + public void testDisallowConfigVpn_clearsUserVpn() throws Exception { + final String userVpnPackage = "org.some.vpn.servcie"; + final int userVpnUid = 20374; + + setDeviceOwner(); + + setupVpnAuthorization(userVpnPackage, userVpnUid); + + simulateRestrictionAdded(UserManager.DISALLOW_CONFIG_VPN); + + verify(getServices().vpnManager).setAlwaysOnVpnPackageForUser( + UserHandle.USER_SYSTEM, null, false, null); + verify(getServices().appOpsManager).setMode(OP_ACTIVATE_VPN, + userVpnUid, userVpnPackage, MODE_DEFAULT); + } + + @Test + public void testDisallowConfigVpn_doesntKillAdminVpn() throws Exception { + setDeviceOwner(); + + when(getServices().vpnManager + .setAlwaysOnVpnPackageForUser(anyInt(), any(), anyBoolean(), any())) + .thenReturn(true); + + // Set VPN package to admin package. + dpm.setAlwaysOnVpnPackage(admin1, admin1.getPackageName(), false, null); + setupVpnAuthorization(admin1.getPackageName(), DpmMockContext.CALLER_SYSTEM_USER_UID); + clearInvocations(getServices().vpnManager); + + simulateRestrictionAdded(UserManager.DISALLOW_CONFIG_VPN); + + // Admin-set package should remain always-on and should retain its authorization. + verifyNoMoreInteractions(getServices().vpnManager); + verify(getServices().appOpsManager, never()).setMode(OP_ACTIVATE_VPN, + DpmMockContext.CALLER_SYSTEM_USER_UID, admin1.getPackageName(), MODE_DEFAULT); + } + + private void setupVpnAuthorization(String userVpnPackage, int userVpnUid) { + final AppOpsManager.PackageOps vpnOp = new AppOpsManager.PackageOps(userVpnPackage, + userVpnUid, List.of(new AppOpsManager.OpEntry( + OP_ACTIVATE_VPN, MODE_ALLOWED, Collections.emptyMap()))); + when(getServices().appOpsManager.getPackagesForOps(any(int[].class))) + .thenReturn(List.of(vpnOp)); + } + + private void simulateRestrictionAdded(String restriction) { + RestrictionsListener listener = new RestrictionsListener( + mServiceContext, getServices().userManagerInternal, dpms); + + final Bundle newRestrictions = new Bundle(); + newRestrictions.putBoolean(restriction, true); + listener.onUserRestrictionsChanged(UserHandle.USER_SYSTEM, newRestrictions, new Bundle()); + } + private void setUserUnlocked(int userHandle, boolean unlocked) { when(getServices().userManager.isUserUnlocked(eq(userHandle))).thenReturn(unlocked); } diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DpmMockContext.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DpmMockContext.java index 2e004683acdbd..d10419d4bfc50 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DpmMockContext.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DpmMockContext.java @@ -230,6 +230,8 @@ public class DpmMockContext extends MockContext { return mMockSystemServices.appOpsManager; case Context.CROSS_PROFILE_APPS_SERVICE: return mMockSystemServices.crossProfileApps; + case Context.VPN_MANAGEMENT_SERVICE: + return mMockSystemServices.vpnManager; } throw new UnsupportedOperationException(); } diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/MockSystemServices.java b/services/tests/servicestests/src/com/android/server/devicepolicy/MockSystemServices.java index 9cc057252a4c5..8a2919d552167 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/MockSystemServices.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/MockSystemServices.java @@ -50,6 +50,7 @@ import android.media.IAudioService; import android.net.ConnectivityManager; import android.net.IIpConnectivityMetrics; import android.net.Uri; +import android.net.VpnManager; import android.net.wifi.WifiManager; import android.os.Handler; import android.os.PowerManager; @@ -123,6 +124,7 @@ public class MockSystemServices { public final PersistentDataBlockManagerInternal persistentDataBlockManagerInternal; public final AppOpsManager appOpsManager; public final UsbManager usbManager; + public final VpnManager vpnManager; /** Note this is a partial mock, not a real mock. */ public final PackageManager packageManager; public final BuildMock buildMock = new BuildMock(); @@ -169,6 +171,7 @@ public class MockSystemServices { persistentDataBlockManagerInternal = mock(PersistentDataBlockManagerInternal.class); appOpsManager = mock(AppOpsManager.class); usbManager = mock(UsbManager.class); + vpnManager = mock(VpnManager.class); // Package manager is huge, so we use a partial mock instead. packageManager = spy(realContext.getPackageManager());