From 6d9fd406d439860b0d18d4f6909d9c4132a2b367 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 29 Jan 2021 20:16:51 +0900 Subject: [PATCH 1/2] Make testLegacyLockdownVpn more realistic. Bug: 173331190 Test: test-only change Change-Id: I81b5686244f479d967c826e29eba4feb396a09cf --- .../server/ConnectivityServiceTest.java | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index b0cc7f1361..07bfbfc27a 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -7160,6 +7160,13 @@ public class ConnectivityServiceTest { when(mKeyStore.get(Credentials.VPN + profileName)).thenReturn(encodedProfile); } + private void establishLegacyLockdownVpn() throws Exception { + // The legacy lockdown VPN only supports userId 0. + final Set ranges = Collections.singleton(UidRange.createForUser(PRIMARY_USER)); + mMockVpn.registerAgent(ranges); + mMockVpn.connect(true); + } + @Test public void testLegacyLockdownVpn() throws Exception { mServiceContext.setPermission( @@ -7254,22 +7261,30 @@ public class ConnectivityServiceTest { mMockVpn.expectStartLegacyVpnRunner(); b1 = expectConnectivityAction(TYPE_VPN, DetailedState.CONNECTED); ExpectedBroadcast b2 = expectConnectivityAction(TYPE_MOBILE, DetailedState.CONNECTED); - mMockVpn.establishForMyUid(); + establishLegacyLockdownVpn(); callback.expectAvailableThenValidatedCallbacks(mMockVpn); defaultCallback.expectAvailableThenValidatedCallbacks(mMockVpn); + NetworkCapabilities vpnNc = mCm.getNetworkCapabilities(mMockVpn.getNetwork()); b1.expectBroadcast(); b2.expectBroadcast(); assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED); assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED); assertNetworkInfo(TYPE_WIFI, DetailedState.DISCONNECTED); assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED); + assertTrue(vpnNc.hasTransport(TRANSPORT_VPN)); + assertTrue(vpnNc.hasTransport(TRANSPORT_CELLULAR)); + assertFalse(vpnNc.hasTransport(TRANSPORT_WIFI)); + assertFalse(vpnNc.hasCapability(NET_CAPABILITY_NOT_METERED)); // Switch default network from cell to wifi. Expect VPN to disconnect and reconnect. final LinkProperties wifiLp = new LinkProperties(); wifiLp.setInterfaceName("wlan0"); wifiLp.addLinkAddress(new LinkAddress("192.0.2.163/25")); wifiLp.addRoute(new RouteInfo(new IpPrefix("0.0.0.0/0"), null, "wlan0")); - mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI, wifiLp); + final NetworkCapabilities wifiNc = new NetworkCapabilities(); + wifiNc.addTransportType(TRANSPORT_WIFI); + wifiNc.addCapability(NET_CAPABILITY_NOT_METERED); + mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI, wifiLp, wifiNc); b1 = expectConnectivityAction(TYPE_MOBILE, DetailedState.DISCONNECTED); // Wifi is CONNECTING because the VPN isn't up yet. @@ -7302,16 +7317,20 @@ public class ConnectivityServiceTest { // The VPN comes up again on wifi. b1 = expectConnectivityAction(TYPE_VPN, DetailedState.CONNECTED); b2 = expectConnectivityAction(TYPE_WIFI, DetailedState.CONNECTED); - mMockVpn.establishForMyUid(); + establishLegacyLockdownVpn(); callback.expectAvailableThenValidatedCallbacks(mMockVpn); defaultCallback.expectAvailableThenValidatedCallbacks(mMockVpn); b1.expectBroadcast(); b2.expectBroadcast(); - assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED); assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED); + vpnNc = mCm.getNetworkCapabilities(mMockVpn.getNetwork()); + assertTrue(vpnNc.hasTransport(TRANSPORT_VPN)); + assertTrue(vpnNc.hasTransport(TRANSPORT_WIFI)); + assertFalse(vpnNc.hasTransport(TRANSPORT_CELLULAR)); + assertTrue(vpnNc.hasCapability(NET_CAPABILITY_NOT_METERED)); // Disconnect cell. Nothing much happens since it's not the default network. // Whenever LockdownVpnTracker is connected, it will send a connected broadcast any time any From dd53c5889b60b1e0d22cb6f441e45bb7e7bca138 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Wed, 3 Feb 2021 02:40:47 +0900 Subject: [PATCH 2/2] Fix the legacy VPN tests that start racoon. These tests do not actually work, or at least not as designed. What happens when they are run is that creating/connecting the VPN throws an exception. The call to legacyRunnerReady.block() times out after 10 seconds because the condition variable is never opened, but the doesn't notice the timeout becasue it doesn't check the return value of block(). There are two reasons for the VPN not coming up. 1. VpnConfig.getIntentForStatusPanel calls into PendingIntent and ActivityManager statics, which bypass all the mocks and fail with an appops denial. Fix this by making it a dependency. 2. The tests are broken due to the UserManager API changes tracked in b/175883995. Fixing by adding a bit of ad-hoc code into startLegacyVpn, with a TODO to delete it once the rest of the UserManager setup code is fixed. Fix these and check the return value of block(). This ensures that if any other breakage is added the test will actually fail. Also check that the throw route survives all the way to the LinkProperties sent to the agent. Bug: 173331190 Test: atest com.android.server.connectivity.VpnTest Change-Id: Ieb7f33bce283ac5ee562a912df8edb9c930ed2b0 --- .../android/server/connectivity/VpnTest.java | 44 ++++++++++++++----- 1 file changed, 32 insertions(+), 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 f4782829cf..3a93c5b105 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -49,6 +49,7 @@ import android.annotation.NonNull; import android.annotation.UserIdInt; import android.app.AppOpsManager; import android.app.NotificationManager; +import android.app.PendingIntent; import android.content.Context; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; @@ -119,6 +120,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; @@ -213,6 +215,8 @@ public class VpnTest { when(mContext.getPackageName()).thenReturn(TEST_VPN_PKG); when(mContext.getOpPackageName()).thenReturn(TEST_VPN_PKG); + when(mContext.getSystemServiceName(UserManager.class)) + .thenReturn(Context.USER_SERVICE); when(mContext.getSystemService(eq(Context.USER_SERVICE))).thenReturn(mUserManager); when(mContext.getSystemService(eq(Context.APP_OPS_SERVICE))).thenReturn(mAppOps); when(mContext.getSystemServiceName(NotificationManager.class)) @@ -954,7 +958,14 @@ public class VpnTest { } private Vpn startLegacyVpn(final Vpn vpn, final VpnProfile vpnProfile) throws Exception { - setMockedUsers(primaryUser); + // 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); // Dummy egress interface final LinkProperties lp = new LinkProperties(); @@ -997,14 +1008,12 @@ public class VpnTest { profile.ipsecIdentifier = "id"; profile.ipsecSecret = "secret"; profile.l2tpSecret = "l2tpsecret"; + when(mConnectivityManager.getAllNetworks()) .thenReturn(new Network[] { new Network(101) }); + when(mConnectivityManager.registerNetworkAgent(any(), any(), any(), any(), - anyInt(), any(), anyInt())).thenAnswer(invocation -> { - // The runner has registered an agent and is now ready. - legacyRunnerReady.open(); - return new Network(102); - }); + anyInt(), any(), anyInt())).thenReturn(new Network(102)); final Vpn vpn = startLegacyVpn(createVpn(primaryUser.id), profile); final TestDeps deps = (TestDeps) vpn.mDeps; try { @@ -1020,14 +1029,20 @@ public class VpnTest { "linkname", "vpn", "refuse-eap", "nodefaultroute", "usepeerdns", "idle", "1800", "mtu", "1270", "mru", "1270" }, deps.mtpdArgs.get(10, TimeUnit.SECONDS)); + // Now wait for the runner to be ready before testing for the route. - legacyRunnerReady.block(10_000); - // In this test the expected address is always v4 so /32 + ArgumentCaptor lpCaptor = ArgumentCaptor.forClass(LinkProperties.class); + verify(mConnectivityManager, timeout(10_000)).registerNetworkAgent(any(), any(), + lpCaptor.capture(), any(), anyInt(), any(), anyInt()); + + // In this test the expected address is always v4 so /32. + // Note that the interface needs to be specified because RouteInfo objects stored in + // LinkProperties objects always acquire the LinkProperties' interface. final RouteInfo expectedRoute = new RouteInfo(new IpPrefix(expectedAddr + "/32"), - RouteInfo.RTN_THROW); - assertTrue("Routes lack the expected throw route (" + expectedRoute + ") : " - + vpn.mConfig.routes, - vpn.mConfig.routes.contains(expectedRoute)); + null, EGRESS_IFACE, RouteInfo.RTN_THROW); + final List actualRoutes = lpCaptor.getValue().getRoutes(); + assertTrue("Expected throw route (" + expectedRoute + ") not found in " + actualRoutes, + actualRoutes.contains(expectedRoute)); } finally { // Now interrupt the thread, unblock the runner and clean up. vpn.mVpnRunner.exitVpnRunner(); @@ -1082,6 +1097,11 @@ public class VpnTest { return mStateFile; } + @Override + public PendingIntent getIntentForStatusPanel(Context context) { + return null; + } + @Override public void sendArgumentsToDaemon( final String daemon, final LocalSocket socket, final String[] arguments,