From e9192a51f43ba04a4a155964fa92d155237fa0cd Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Sat, 6 Feb 2021 01:18:40 +0900 Subject: [PATCH 1/3] Remove temporary hack in VpnTest. This temporary hack was necessary due to b/175883995. Now that that bug is fixed, remove the hack. Bug: 171529940 Test: test-only change Change-Id: I48bd594cf3a37575f4888c1be9a1cf535de2e4d2 --- .../com/android/server/connectivity/VpnTest.java | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index 32c6a75bd9..73cc9f129e 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -953,14 +953,7 @@ public class VpnTest { } private Vpn startLegacyVpn(final Vpn vpn, final VpnProfile vpnProfile) throws Exception { - // TODO(b/175883995): once these tests have been updated for the changes to the UserManager - // API, remove this ad-hoc setup code and use setMockedUsers(primaryUser) again. - // setMockedUsers(primaryUser); - final ArrayList users = new ArrayList<>(); - users.add(primaryUser); - when(mUserManager.getAliveUsers()).thenReturn(users); - when(mUserManager.getUserInfo(primaryUser.id)).thenReturn(primaryUser); - when(mUserManager.canHaveRestrictedProfile()).thenReturn(false); + setMockedUsers(primaryUser); // Dummy egress interface final LinkProperties lp = new LinkProperties(); @@ -1159,10 +1152,6 @@ public class VpnTest { doReturn(UserHandle.of(userId)).when(asUserContext).getUser(); when(mContext.createContextAsUser(eq(UserHandle.of(userId)), anyInt())) .thenReturn(asUserContext); - when(asUserContext.getSystemServiceName(UserManager.class)) - .thenReturn(Context.USER_SERVICE); - when(asUserContext.getSystemService(UserManager.class)) - .thenReturn(mUserManager); final TestLooper testLooper = new TestLooper(); final Vpn vpn = new Vpn(testLooper.getLooper(), mContext, new TestDeps(), mNetService, mNetd, userId, mKeyStore, mSystemServices, mIkev2SessionCreator); From f80ad439dd87faa05552c7f6d63f35dc6a52bf3e Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 5 Feb 2021 01:10:56 +0900 Subject: [PATCH 2/3] Add to VpnManager the VPN APIs used by Settings and SystemUI. Settings and SystemUI use these through raw AIDL calls. Make them proper manager calls so we can move the implementation without touching the callers again. For now these still call into ConnectivityService via IConnectivityManager. In an upcoming CL the implementation will move to VpnManagerService. Test: m Bug: 173331190 Change-Id: I91528e1ad9948fbaa1fc5e37c61c5eb95f54964e --- framework/src/android/net/VpnManager.java | 105 +++++++++++++++++++++- 1 file changed, 104 insertions(+), 1 deletion(-) diff --git a/framework/src/android/net/VpnManager.java b/framework/src/android/net/VpnManager.java index c87b8279c4..1812509ba6 100644 --- a/framework/src/android/net/VpnManager.java +++ b/framework/src/android/net/VpnManager.java @@ -21,6 +21,7 @@ import static com.android.internal.util.Preconditions.checkNotNull; import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; +import android.annotation.UserIdInt; import android.app.Activity; import android.content.ComponentName; import android.content.Context; @@ -28,6 +29,8 @@ import android.content.Intent; import android.content.res.Resources; import android.os.RemoteException; +import com.android.internal.net.LegacyVpnInfo; +import com.android.internal.net.VpnConfig; import com.android.internal.net.VpnProfile; import java.io.IOException; @@ -161,4 +164,104 @@ public class VpnManager { throw e.rethrowFromSystemServer(); } } -} + + /** + * Return the VPN configuration for the given user ID. + * @hide + */ + @Nullable + public VpnConfig getVpnConfig(@UserIdInt int userId) { + try { + return mService.getVpnConfig(userId); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + + /** + * Prepare for a VPN application. + * VPN permissions are checked in the {@link Vpn} class. If the caller is not {@code userId}, + * {@link android.Manifest.permission.INTERACT_ACROSS_USERS_FULL} permission is required. + * + * @param oldPackage Package name of the application which currently controls VPN, which will + * be replaced. If there is no such application, this should should either be + * {@code null} or {@link VpnConfig.LEGACY_VPN}. + * @param newPackage Package name of the application which should gain control of VPN, or + * {@code null} to disable. + * @param userId User for whom to prepare the new VPN. + * + * @hide + */ + public boolean prepareVpn(@Nullable String oldPackage, @Nullable String newPackage, + int userId) { + try { + return mService.prepareVpn(oldPackage, newPackage, userId); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + + /** + * Set whether the VPN package has the ability to launch VPNs without user intervention. This + * method is used by system-privileged apps. VPN permissions are checked in the {@link Vpn} + * class. If the caller is not {@code userId}, {@link + * android.Manifest.permission.INTERACT_ACROSS_USERS_FULL} permission is required. + * + * @param packageName The package for which authorization state should change. + * @param userId User for whom {@code packageName} is installed. + * @param vpnType The {@link VpnManager.VpnType} constant representing what class of VPN + * permissions should be granted. When unauthorizing an app, {@link + * VpnManager.TYPE_VPN_NONE} should be used. + * @hide + */ + public void setVpnPackageAuthorization( + String packageName, int userId, @VpnManager.VpnType int vpnType) { + try { + mService.setVpnPackageAuthorization(packageName, userId, vpnType); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + + /** + * Return the legacy VPN information for the specified user ID. + * @hide + */ + public LegacyVpnInfo getLegacyVpnInfo(@UserIdInt int userId) { + try { + return mService.getLegacyVpnInfo(userId); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + + /** + * Starts a legacy VPN. + * @hide + */ + public void startLegacyVpn(VpnProfile profile) { + try { + mService.startLegacyVpn(profile); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + + /** + * Informs the service that legacy lockdown VPN state should be updated (e.g., if its keystore + * entry has been updated). If the LockdownVpn mechanism is enabled, updates the vpn + * with a reload of its profile. + * + *

This method can only be called by the system UID + * @return a boolean indicating success + * + * @hide + */ + public boolean updateLockdownVpn() { + try { + return mService.updateLockdownVpn(); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } +} \ No newline at end of file From af17de03cee9dcc3ce0c190bcb2487424a07fb4c Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Sat, 6 Feb 2021 01:14:29 +0900 Subject: [PATCH 3/3] Change ConnectivityServiceTest to prepare for VPN code move. No-op changes to make it easier to review the VPN code move. 1. Set underlying networks via MockVpn, not ConnectivityService. 2. Add a processBroadcastForVpn utility method that we can modify without too much churn when the code is moved. Bug: 173331190 Test: test-only change Change-Id: I4256ca5f33034ef9d1de79857795a6ed1d644e61 --- .../server/ConnectivityServiceTest.java | 74 +++++++++---------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 9f48535515..024e6e318b 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -1262,6 +1262,13 @@ public class ConnectivityServiceTest { } } + private void processBroadcastForVpn(Intent intent) { + // The BroadcastReceiver for this broadcast checks it is being run on the handler thread. + final Handler handler = new Handler(mCsHandlerThread.getLooper()); + handler.post(() -> mServiceContext.sendBroadcast(intent)); + waitForIdle(); + } + private void mockUidNetworkingBlocked() { doAnswer(i -> mContext.getSystemService(NetworkPolicyManager.class) .checkUidNetworkingBlocked(i.getArgument(0) /* uid */, mUidRules, @@ -5300,20 +5307,20 @@ public class ConnectivityServiceTest { // MOBILE_IFNAME even though the default network is wifi. // TODO: fix this to pass in the actual default network interface. Whether or not the VPN // applies to the system server UID should not have any bearing on network stats. - mService.setUnderlyingNetworksForVpn(onlyCell); + mMockVpn.setUnderlyingNetworks(onlyCell); waitForIdle(); expectForceUpdateIfaces(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME, new String[]{MOBILE_IFNAME}); reset(mStatsService); - mService.setUnderlyingNetworksForVpn(cellAndWifi); + mMockVpn.setUnderlyingNetworks(cellAndWifi); waitForIdle(); expectForceUpdateIfaces(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME, new String[]{MOBILE_IFNAME, WIFI_IFNAME}); reset(mStatsService); // Null underlying networks are ignored. - mService.setUnderlyingNetworksForVpn(cellNullAndWifi); + mMockVpn.setUnderlyingNetworks(cellNullAndWifi); waitForIdle(); expectForceUpdateIfaces(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME, new String[]{MOBILE_IFNAME, WIFI_IFNAME}); @@ -5362,25 +5369,25 @@ public class ConnectivityServiceTest { // is probably a performance improvement (though it's very unlikely that a VPN would declare // no underlying networks). // Also, for the same reason as above, the active interface passed in is null. - mService.setUnderlyingNetworksForVpn(new Network[0]); + mMockVpn.setUnderlyingNetworks(new Network[0]); waitForIdle(); expectForceUpdateIfaces(wifiAndVpn, null); reset(mStatsService); // Specifying only a null underlying network is the same as no networks. - mService.setUnderlyingNetworksForVpn(onlyNull); + mMockVpn.setUnderlyingNetworks(onlyNull); waitForIdle(); expectForceUpdateIfaces(wifiAndVpn, null); reset(mStatsService); // Specifying networks that are all disconnected is the same as specifying no networks. - mService.setUnderlyingNetworksForVpn(onlyCell); + mMockVpn.setUnderlyingNetworks(onlyCell); waitForIdle(); expectForceUpdateIfaces(wifiAndVpn, null); reset(mStatsService); // Passing in null again means follow the default network again. - mService.setUnderlyingNetworksForVpn(null); + mMockVpn.setUnderlyingNetworks(null); waitForIdle(); expectForceUpdateIfaces(wifiAndVpn, WIFI_IFNAME, Process.myUid(), VPN_IFNAME, new String[]{WIFI_IFNAME}); @@ -5855,7 +5862,7 @@ public class ConnectivityServiceTest { mMockVpn.establishForMyUid(false, true, false); assertUidRangesUpdatedForMyUid(true); final Network wifiNetwork = new Network(mNetIdManager.peekNextNetId()); - mService.setUnderlyingNetworksForVpn(new Network[]{wifiNetwork}); + mMockVpn.setUnderlyingNetworks(new Network[]{wifiNetwork}); callback.expectAvailableCallbacksUnvalidated(mMockVpn); assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork()) .hasTransport(TRANSPORT_VPN)); @@ -6049,7 +6056,7 @@ public class ConnectivityServiceTest { final Set ranges = uidRangesForUid(uid); mMockVpn.registerAgent(ranges); - mService.setUnderlyingNetworksForVpn(new Network[0]); + mMockVpn.setUnderlyingNetworks(new Network[0]); // VPN networks do not satisfy the default request and are automatically validated // by NetworkMonitor @@ -6297,7 +6304,7 @@ public class ConnectivityServiceTest { mCellNetworkAgent.addCapability(NET_CAPABILITY_NOT_SUSPENDED); mCellNetworkAgent.connect(true); - mService.setUnderlyingNetworksForVpn( + mMockVpn.setUnderlyingNetworks( new Network[] { mCellNetworkAgent.getNetwork() }); vpnNetworkCallback.expectCapabilitiesThat(mMockVpn, @@ -6312,7 +6319,7 @@ public class ConnectivityServiceTest { mWiFiNetworkAgent.addCapability(NET_CAPABILITY_NOT_SUSPENDED); mWiFiNetworkAgent.connect(true); - mService.setUnderlyingNetworksForVpn( + mMockVpn.setUnderlyingNetworks( new Network[] { mCellNetworkAgent.getNetwork(), mWiFiNetworkAgent.getNetwork() }); vpnNetworkCallback.expectCapabilitiesThat(mMockVpn, @@ -6323,7 +6330,7 @@ public class ConnectivityServiceTest { assertDefaultNetworkCapabilities(userId, mCellNetworkAgent, mWiFiNetworkAgent); // Don't disconnect, but note the VPN is not using wifi any more. - mService.setUnderlyingNetworksForVpn( + mMockVpn.setUnderlyingNetworks( new Network[] { mCellNetworkAgent.getNetwork() }); vpnNetworkCallback.expectCapabilitiesThat(mMockVpn, @@ -6354,7 +6361,7 @@ public class ConnectivityServiceTest { vpnNetworkCallback.expectCallback(CallbackEntry.RESUMED, mMockVpn); // Use Wifi but not cell. Note the VPN is now unmetered and not suspended. - mService.setUnderlyingNetworksForVpn( + mMockVpn.setUnderlyingNetworks( new Network[] { mWiFiNetworkAgent.getNetwork() }); vpnNetworkCallback.expectCapabilitiesThat(mMockVpn, @@ -6365,7 +6372,7 @@ public class ConnectivityServiceTest { assertDefaultNetworkCapabilities(userId, mWiFiNetworkAgent); // Use both again. - mService.setUnderlyingNetworksForVpn( + mMockVpn.setUnderlyingNetworks( new Network[] { mCellNetworkAgent.getNetwork(), mWiFiNetworkAgent.getNetwork() }); vpnNetworkCallback.expectCapabilitiesThat(mMockVpn, @@ -6380,7 +6387,7 @@ public class ConnectivityServiceTest { vpnNetworkCallback.assertNoCallback(); // Stop using WiFi. The VPN is suspended again. - mService.setUnderlyingNetworksForVpn( + mMockVpn.setUnderlyingNetworks( new Network[] { mCellNetworkAgent.getNetwork() }); vpnNetworkCallback.expectCapabilitiesThat(mMockVpn, (caps) -> caps.hasTransport(TRANSPORT_VPN) @@ -6391,7 +6398,7 @@ public class ConnectivityServiceTest { assertDefaultNetworkCapabilities(userId, mCellNetworkAgent, mWiFiNetworkAgent); // Use both again. - mService.setUnderlyingNetworksForVpn( + mMockVpn.setUnderlyingNetworks( new Network[] { mCellNetworkAgent.getNetwork(), mWiFiNetworkAgent.getNetwork() }); vpnNetworkCallback.expectCapabilitiesThat(mMockVpn, @@ -6526,9 +6533,7 @@ public class ConnectivityServiceTest { addedIntent.putExtra(Intent.EXTRA_USER_HANDLE, RESTRICTED_USER); // Send a USER_ADDED broadcast for it. - // The BroadcastReceiver for this broadcast checks that is being run on the handler thread. - final Handler handler = new Handler(mCsHandlerThread.getLooper()); - handler.post(() -> mServiceContext.sendBroadcast(addedIntent)); + processBroadcastForVpn(addedIntent); // Expect that the VPN UID ranges contain both |uid| and the UID range for the newly-added // restricted user. @@ -6552,7 +6557,7 @@ public class ConnectivityServiceTest { // Send a USER_REMOVED broadcast and expect to lose the UID range for the restricted user. final Intent removedIntent = new Intent(ACTION_USER_REMOVED); removedIntent.putExtra(Intent.EXTRA_USER_HANDLE, RESTRICTED_USER); - handler.post(() -> mServiceContext.sendBroadcast(removedIntent)); + processBroadcastForVpn(removedIntent); // Expect that the VPN gains the UID range for the restricted user, and that the capability // change made just before that (i.e., loss of TRANSPORT_WIFI) is preserved. @@ -6609,9 +6614,7 @@ public class ConnectivityServiceTest { // TODO: check that VPN app within restricted profile still has access, etc. final Intent addedIntent = new Intent(ACTION_USER_ADDED); addedIntent.putExtra(Intent.EXTRA_USER_HANDLE, RESTRICTED_USER); - final Handler handler = new Handler(mCsHandlerThread.getLooper()); - handler.post(() -> mServiceContext.sendBroadcast(addedIntent)); - waitForIdle(); + processBroadcastForVpn(addedIntent); assertNull(mCm.getActiveNetworkForUid(uid)); assertNull(mCm.getActiveNetworkForUid(restrictedUid)); @@ -6621,8 +6624,7 @@ public class ConnectivityServiceTest { // Send a USER_REMOVED broadcast and expect to lose the UID range for the restricted user. final Intent removedIntent = new Intent(ACTION_USER_REMOVED); removedIntent.putExtra(Intent.EXTRA_USER_HANDLE, RESTRICTED_USER); - handler.post(() -> mServiceContext.sendBroadcast(removedIntent)); - waitForIdle(); + processBroadcastForVpn(removedIntent); assertNull(mCm.getActiveNetworkForUid(uid)); assertNotNull(mCm.getActiveNetworkForUid(restrictedUid)); @@ -6724,7 +6726,7 @@ public class ConnectivityServiceTest { // Ensure VPN is now the active network. assertEquals(mMockVpn.getNetwork(), mCm.getActiveNetwork()); // VPN is using Cell - mService.setUnderlyingNetworksForVpn( + mMockVpn.setUnderlyingNetworks( new Network[] { mCellNetworkAgent.getNetwork() }); waitForIdle(); @@ -6732,7 +6734,7 @@ public class ConnectivityServiceTest { assertTrue(mCm.isActiveNetworkMetered()); // VPN is now using WiFi - mService.setUnderlyingNetworksForVpn( + mMockVpn.setUnderlyingNetworks( new Network[] { mWiFiNetworkAgent.getNetwork() }); waitForIdle(); @@ -6740,7 +6742,7 @@ public class ConnectivityServiceTest { assertFalse(mCm.isActiveNetworkMetered()); // VPN is using Cell | WiFi. - mService.setUnderlyingNetworksForVpn( + mMockVpn.setUnderlyingNetworks( new Network[] { mCellNetworkAgent.getNetwork(), mWiFiNetworkAgent.getNetwork() }); waitForIdle(); @@ -6748,7 +6750,7 @@ public class ConnectivityServiceTest { assertTrue(mCm.isActiveNetworkMetered()); // VPN is using WiFi | Cell. - mService.setUnderlyingNetworksForVpn( + mMockVpn.setUnderlyingNetworks( new Network[] { mWiFiNetworkAgent.getNetwork(), mCellNetworkAgent.getNetwork() }); waitForIdle(); @@ -6756,7 +6758,7 @@ public class ConnectivityServiceTest { assertTrue(mCm.isActiveNetworkMetered()); // VPN is not using any underlying networks. - mService.setUnderlyingNetworksForVpn(new Network[0]); + mMockVpn.setUnderlyingNetworks(new Network[0]); waitForIdle(); // VPN without underlying networks is treated as metered. @@ -6783,7 +6785,7 @@ public class ConnectivityServiceTest { assertEquals(mMockVpn.getNetwork(), mCm.getActiveNetwork()); // VPN is tracking current platform default (WiFi). - mService.setUnderlyingNetworksForVpn(null); + mMockVpn.setUnderlyingNetworks(null); waitForIdle(); // Despite VPN using WiFi (which is unmetered), VPN itself is marked as always metered. @@ -6791,7 +6793,7 @@ public class ConnectivityServiceTest { // VPN explicitly declares WiFi as its underlying network. - mService.setUnderlyingNetworksForVpn( + mMockVpn.setUnderlyingNetworks( new Network[] { mWiFiNetworkAgent.getNetwork() }); waitForIdle(); @@ -7199,9 +7201,7 @@ public class ConnectivityServiceTest { final int userId = UserHandle.getUserId(Process.myUid()); final Intent addedIntent = new Intent(ACTION_USER_UNLOCKED); addedIntent.putExtra(Intent.EXTRA_USER_HANDLE, userId); - final Handler handler = new Handler(mCsHandlerThread.getLooper()); - handler.post(() -> mServiceContext.sendBroadcast(addedIntent)); - waitForIdle(); + processBroadcastForVpn(addedIntent); // Lockdown VPN disables teardown and enables lockdown. assertFalse(mMockVpn.getEnableTeardown()); @@ -8643,7 +8643,7 @@ public class ConnectivityServiceTest { setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION); - assertTrue(mService.setUnderlyingNetworksForVpn(new Network[] {naiWithoutUid.network})); + assertTrue(mMockVpn.setUnderlyingNetworks(new Network[] {naiWithoutUid.network})); waitForIdle(); assertTrue( "Active VPN permission not applied", @@ -8651,7 +8651,7 @@ public class ConnectivityServiceTest { Process.myPid(), Process.myUid(), naiWithoutUid, mContext.getOpPackageName())); - assertTrue(mService.setUnderlyingNetworksForVpn(null)); + assertTrue(mMockVpn.setUnderlyingNetworks(null)); waitForIdle(); assertFalse( "VPN shouldn't receive callback on non-underlying network",