From 746cdc28af26ec614ab7cf13e384c549c24de37d Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 9 Oct 2020 15:58:41 +0900 Subject: [PATCH 1/2] Migrate VPN to the public NetworkAgent API. On top of being a cleanup this is useful for the S Network Selection project that will need to enrich the Network Agent API, and as such should not have to support legacy agents. Test: FrameworksNetTests NetworkStackTests Bug: 167544279 Change-Id: Id3e5f6e19829c64074cd6a52c5f950cee56b860b --- core/java/android/net/NetworkProvider.java | 7 ------- .../server/ConnectivityServiceTest.java | 8 ++++++-- .../android/server/connectivity/VpnTest.java | 19 +++++++++---------- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/core/java/android/net/NetworkProvider.java b/core/java/android/net/NetworkProvider.java index d31218d9b6..a17a49897d 100644 --- a/core/java/android/net/NetworkProvider.java +++ b/core/java/android/net/NetworkProvider.java @@ -50,13 +50,6 @@ public class NetworkProvider { */ public static final int ID_NONE = -1; - /** - * A hardcoded ID for NetworkAgents representing VPNs. These agents are not created by any - * provider, so they use this constant for clarity instead of NONE. - * @hide only used by ConnectivityService. - */ - public static final int ID_VPN = -2; - /** * The first providerId value that will be allocated. * @hide only used by ConnectivityService. diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 561c6bab9c..7e8f195885 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -1089,6 +1089,10 @@ public class ConnectivityServiceTest { mMockNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_VPN, lp, mNetworkCapabilities); mMockNetworkAgent.waitForIdle(TIMEOUT_MS); + verify(mNetworkManagementService, times(1)) + .addVpnUidRanges(eq(mMockVpn.getNetId()), eq(uids.toArray(new UidRange[0]))); + verify(mNetworkManagementService, never()) + .removeVpnUidRanges(eq(mMockVpn.getNetId()), any()); mAgentRegistered = true; mNetworkCapabilities.set(mMockNetworkAgent.getNetworkCapabilities()); mNetworkAgent = mMockNetworkAgent.getNetworkAgent(); @@ -6922,8 +6926,8 @@ public class ConnectivityServiceTest { final Set vpnRange = Collections.singleton(UidRange.createForUser(VPN_USER)); mMockVpn.establish(lp, VPN_UID, vpnRange); - // Connected VPN should have interface rules set up. There are two expected invocations, - // one during VPN uid update, one during VPN LinkProperties update + // A connected VPN should have interface rules set up. There are two expected invocations, + // one during the VPN initial connection, one during the VPN LinkProperties update. ArgumentCaptor uidCaptor = ArgumentCaptor.forClass(int[].class); verify(mMockNetd, times(2)).firewallAddUidInterfaceRules(eq("tun0"), uidCaptor.capture()); assertContainsExactly(uidCaptor.getAllValues().get(0), APP1_UID, APP2_UID); diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index a553b584a2..11bf7a99ee 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -100,6 +100,7 @@ import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; import com.android.internal.R; +import com.android.internal.net.LegacyVpnInfo; import com.android.internal.net.VpnConfig; import com.android.internal.net.VpnProfile; import com.android.server.IpSecService; @@ -589,7 +590,7 @@ public class VpnTest { } @Test - public void testNotificationShownForAlwaysOnApp() { + public void testNotificationShownForAlwaysOnApp() throws Exception { final UserHandle userHandle = UserHandle.of(primaryUser.id); final Vpn vpn = createVpn(primaryUser.id); setMockedUsers(primaryUser); @@ -619,7 +620,6 @@ public class VpnTest { @Test public void testCapabilities() { - final Vpn vpn = createVpn(primaryUser.id); setMockedUsers(primaryUser); final Network mobile = new Network(1); @@ -1037,7 +1037,7 @@ public class VpnTest { when(exception.getErrorType()) .thenReturn(IkeProtocolException.ERROR_TYPE_AUTHENTICATION_FAILED); - final Vpn vpn = startLegacyVpn(mVpnProfile); + final Vpn vpn = startLegacyVpn(createVpn(primaryUser.id), (mVpnProfile)); final NetworkCallback cb = triggerOnAvailableAndGetCallback(); // Wait for createIkeSession() to be called before proceeding in order to ensure consistent @@ -1048,20 +1048,20 @@ public class VpnTest { ikeCb.onClosedExceptionally(exception); verify(mConnectivityManager, timeout(TEST_TIMEOUT_MS)).unregisterNetworkCallback(eq(cb)); - assertEquals(DetailedState.FAILED, vpn.getNetworkInfo().getDetailedState()); + assertEquals(LegacyVpnInfo.STATE_FAILED, vpn.getLegacyVpnInfo().state); } @Test public void testStartPlatformVpnIllegalArgumentExceptionInSetup() throws Exception { when(mIkev2SessionCreator.createIkeSession(any(), any(), any(), any(), any(), any())) .thenThrow(new IllegalArgumentException()); - final Vpn vpn = startLegacyVpn(mVpnProfile); + final Vpn vpn = startLegacyVpn(createVpn(primaryUser.id), mVpnProfile); final NetworkCallback cb = triggerOnAvailableAndGetCallback(); // Wait for createIkeSession() to be called before proceeding in order to ensure consistent // state verify(mConnectivityManager, timeout(TEST_TIMEOUT_MS)).unregisterNetworkCallback(eq(cb)); - assertEquals(DetailedState.FAILED, vpn.getNetworkInfo().getDetailedState()); + assertEquals(LegacyVpnInfo.STATE_FAILED, vpn.getLegacyVpnInfo().state); } private void setAndVerifyAlwaysOnPackage(Vpn vpn, int uid, boolean lockdownEnabled) { @@ -1100,8 +1100,7 @@ public class VpnTest { // a subsequent CL. } - public Vpn startLegacyVpn(final VpnProfile vpnProfile) throws Exception { - final Vpn vpn = createVpn(primaryUser.id); + private Vpn startLegacyVpn(final Vpn vpn, final VpnProfile vpnProfile) throws Exception { setMockedUsers(primaryUser); // Dummy egress interface @@ -1118,7 +1117,7 @@ public class VpnTest { @Test public void testStartPlatformVpn() throws Exception { - startLegacyVpn(mVpnProfile); + startLegacyVpn(createVpn(primaryUser.id), mVpnProfile); // TODO: Test the Ikev2VpnRunner started up properly. Relies on utility methods added in // a subsequent patch. } @@ -1153,7 +1152,7 @@ public class VpnTest { legacyRunnerReady.open(); return new Network(102); }); - final Vpn vpn = startLegacyVpn(profile); + final Vpn vpn = startLegacyVpn(createVpn(primaryUser.id), profile); final TestDeps deps = (TestDeps) vpn.mDeps; try { // udppsk and 1701 are the values for TYPE_L2TP_IPSEC_PSK From c99d746c06a26e97193e77c671af843109203090 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 26 Nov 2020 23:21:13 +0900 Subject: [PATCH 2/2] Add a provider to VPN Test: FrameworksNetTests NetworkStackTests Change-Id: I982543cdee358bb62d3b56a7fd9d71dc18908b65 --- .../java/com/android/server/ConnectivityService.java | 8 ++++---- .../com/android/server/connectivity/VpnTest.java | 12 ++++++++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 5420ee2f11..d4529740ce 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -5140,7 +5140,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - private void onUserStart(int userId) { + private void onUserStarted(int userId) { synchronized (mVpns) { Vpn userVpn = mVpns.get(userId); if (userVpn != null) { @@ -5155,7 +5155,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - private void onUserStop(int userId) { + private void onUserStopped(int userId) { synchronized (mVpns) { Vpn userVpn = mVpns.get(userId); if (userVpn == null) { @@ -5272,9 +5272,9 @@ public class ConnectivityService extends IConnectivityManager.Stub if (userId == UserHandle.USER_NULL) return; if (Intent.ACTION_USER_STARTED.equals(action)) { - onUserStart(userId); + onUserStarted(userId); } else if (Intent.ACTION_USER_STOPPED.equals(action)) { - onUserStop(userId); + onUserStopped(userId); } else if (Intent.ACTION_USER_ADDED.equals(action)) { onUserAdded(userId); } else if (Intent.ACTION_USER_REMOVED.equals(action)) { diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index 11bf7a99ee..e1e0efa5cf 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -41,6 +41,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doAnswer; @@ -86,10 +87,10 @@ import android.os.Build.VERSION_CODES; import android.os.Bundle; import android.os.ConditionVariable; import android.os.INetworkManagementService; -import android.os.Looper; import android.os.Process; import android.os.UserHandle; import android.os.UserManager; +import android.os.test.TestLooper; import android.provider.Settings; import android.security.Credentials; import android.security.KeyStore; @@ -224,6 +225,8 @@ public class VpnTest { .thenReturn(mNotificationManager); when(mContext.getSystemService(eq(Context.CONNECTIVITY_SERVICE))) .thenReturn(mConnectivityManager); + when(mContext.getSystemServiceName(eq(ConnectivityManager.class))) + .thenReturn(Context.CONNECTIVITY_SERVICE); when(mContext.getSystemService(eq(Context.IPSEC_SERVICE))).thenReturn(mIpSecManager); when(mContext.getString(R.string.config_customVpnAlwaysOnDisconnectedDialogComponent)) .thenReturn(Resources.getSystem().getString( @@ -1286,8 +1289,13 @@ public class VpnTest { doReturn(UserHandle.of(userId)).when(asUserContext).getUser(); when(mContext.createContextAsUser(eq(UserHandle.of(userId)), anyInt())) .thenReturn(asUserContext); - return new Vpn(Looper.myLooper(), mContext, new TestDeps(), mNetService, + final TestLooper testLooper = new TestLooper(); + final Vpn vpn = new Vpn(testLooper.getLooper(), mContext, new TestDeps(), mNetService, userId, mKeyStore, mSystemServices, mIkev2SessionCreator); + verify(mConnectivityManager, times(1)).registerNetworkProvider(argThat( + provider -> provider.getName().contains("VpnNetworkProvider") + )); + return vpn; } private static void assertBlocked(Vpn vpn, int... uids) {