Merge "Revert^2 "Revert "Add CTS tests for exclude VPN routes APIs""" am: 67197b7ed6

Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/1941195

Change-Id: I79923b432c049fb8e2cbd42d97f49e78d0631b24

tests/cts/hostside/Android.bp

@@ -25,9 +25,6 @@ java_test_host {
         "cts-tradefed",
         "tradefed",
     ],
-    static_libs: [
-        "modules-utils-build-testing",
-    ],
     // Tag this module as a cts test artifact
     test_suites: [
         "cts",

tests/cts/hostside/app/Android.bp

@@ -18,8 +18,12 @@ package {
     default_applicable_licenses: ["Android-Apache-2.0"],
 }
 
-java_defaults {
+android_test_helper_app {
-    name: "CtsHostsideNetworkTestsAppDefaults",
+    name: "CtsHostsideNetworkTestsApp",
+    defaults: [
+        "cts_support_defaults",
+        "framework-connectivity-test-defaults",
+    ],
     platform_apis: true,
     static_libs: [
         "CtsHostsideNetworkTestsAidl",
@@ -44,28 +48,3 @@ java_defaults {
         "sts",
     ],
 }
-
-android_test_helper_app {
-    name: "CtsHostsideNetworkTestsApp",
-    defaults: [
-        "cts_support_defaults",
-        "framework-connectivity-test-defaults",
-        "CtsHostsideNetworkTestsAppDefaults",
-    ],
-    static_libs: [
-        "NetworkStackApiStableShims",
-    ],
-}
-
-android_test_helper_app {
-    name: "CtsHostsideNetworkTestsAppNext",
-    defaults: [
-        "cts_support_defaults",
-        "framework-connectivity-test-defaults",
-        "CtsHostsideNetworkTestsAppDefaults",
-        "ConnectivityNextEnableDefaults",
-    ],
-    static_libs: [
-        "NetworkStackApiCurrentShims",
-    ],
-}

tests/cts/hostside/app/src/com/android/cts/net/hostside/MyVpnService.java

@@ -18,26 +18,17 @@ package com.android.cts.net.hostside;
 
 import android.content.Intent;
 import android.content.pm.PackageManager.NameNotFoundException;
-import android.net.IpPrefix;
 import android.net.Network;
-import android.net.NetworkUtils;
 import android.net.ProxyInfo;
 import android.net.VpnService;
 import android.os.ParcelFileDescriptor;
 import android.text.TextUtils;
 import android.util.Log;
-import android.util.Pair;
-
-import com.android.modules.utils.build.SdkLevel;
-import com.android.networkstack.apishim.VpnServiceBuilderShimImpl;
-import com.android.networkstack.apishim.common.UnsupportedApiLevelException;
-import com.android.networkstack.apishim.common.VpnServiceBuilderShim;
 
 import java.io.IOException;
 import java.net.InetAddress;
+import java.net.UnknownHostException;
 import java.util.ArrayList;
-import java.util.function.BiConsumer;
-import java.util.function.Consumer;
 
 public class MyVpnService extends VpnService {
 
@@ -76,62 +67,39 @@ public class MyVpnService extends VpnService {
                 (underlyingNetworks != null) ? underlyingNetworks.toArray(new Network[0]) : null);
     }
 
-    private String parseIpAndMaskListArgument(String packageName, Intent intent, String argName,
-            BiConsumer<InetAddress, Integer> consumer) {
-        final String addresses = intent.getStringExtra(packageName + "." + argName);
-
-        if (TextUtils.isEmpty(addresses)) {
-            return null;
-        }
-
-        final String[] addressesArray = addresses.split(",");
-        for (String address : addressesArray) {
-            final Pair<InetAddress, Integer> ipAndMask = NetworkUtils.parseIpAndMask(address);
-            consumer.accept(ipAndMask.first, ipAndMask.second);
-        }
-
-        return addresses;
-    }
-
-    private String parseIpPrefixListArgument(String packageName, Intent intent, String argName,
-            Consumer<IpPrefix> consumer) {
-        return parseIpAndMaskListArgument(packageName, intent, argName,
-                (inetAddress, prefixLength) -> consumer.accept(
-                        new IpPrefix(inetAddress, prefixLength)));
-    }
-
     private void start(String packageName, Intent intent) {
         Builder builder = new Builder();
-        VpnServiceBuilderShim vpnServiceBuilderShim = VpnServiceBuilderShimImpl.newInstance();
 
-        final String addresses = parseIpAndMaskListArgument(packageName, intent, "addresses",
+        String addresses = intent.getStringExtra(packageName + ".addresses");
-                builder::addAddress);
+        if (addresses != null) {
-
+            String[] addressArray = addresses.split(",");
-        String addedRoutes;
+            for (int i = 0; i < addressArray.length; i++) {
-        if (SdkLevel.isAtLeastT() && intent.getBooleanExtra(packageName + ".addRoutesByIpPrefix",
+                String[] prefixAndMask = addressArray[i].split("/");
-                false)) {
-            addedRoutes = parseIpPrefixListArgument(packageName, intent, "routes", (prefix) -> {
                 try {
-                    vpnServiceBuilderShim.addRoute(builder, prefix);
+                    InetAddress address = InetAddress.getByName(prefixAndMask[0]);
-                } catch (UnsupportedApiLevelException e) {
+                    int prefixLength = Integer.parseInt(prefixAndMask[1]);
-                    throw new RuntimeException(e);
+                    builder.addAddress(address, prefixLength);
+                } catch (UnknownHostException|NumberFormatException|
+                         ArrayIndexOutOfBoundsException e) {
+                    continue;
                 }
-            });
+            }
-        } else {
-            addedRoutes = parseIpAndMaskListArgument(packageName, intent, "routes",
-                    builder::addRoute);
         }
 
-        String excludedRoutes = null;
+        String routes = intent.getStringExtra(packageName + ".routes");
-        if (SdkLevel.isAtLeastT()) {
+        if (routes != null) {
-            excludedRoutes = parseIpPrefixListArgument(packageName, intent, "excludedRoutes",
+            String[] routeArray = routes.split(",");
-                    (prefix) -> {
+            for (int i = 0; i < routeArray.length; i++) {
-                        try {
+                String[] prefixAndMask = routeArray[i].split("/");
-                            vpnServiceBuilderShim.excludeRoute(builder, prefix);
+                try {
-                        } catch (UnsupportedApiLevelException e) {
+                    InetAddress address = InetAddress.getByName(prefixAndMask[0]);
-                            throw new RuntimeException(e);
+                    int prefixLength = Integer.parseInt(prefixAndMask[1]);
-                        }
+                    builder.addRoute(address, prefixLength);
-                    });
+                } catch (UnknownHostException|NumberFormatException|
+                         ArrayIndexOutOfBoundsException e) {
+                    continue;
+                }
+            }
         }
 
         String allowed = intent.getStringExtra(packageName + ".allowedapplications");
@@ -184,8 +152,7 @@ public class MyVpnService extends VpnService {
 
         Log.i(TAG, "Establishing VPN,"
                 + " addresses=" + addresses
-                + " addedRoutes=" + addedRoutes
+                + " routes=" + routes
-                + " excludedRoutes=" + excludedRoutes
                 + " allowedApplications=" + allowed
                 + " disallowedApplications=" + disallowed);
 

tests/cts/hostside/app/src/com/android/cts/net/hostside/VpnTest.java

@@ -298,54 +298,28 @@ public class VpnTest {
         mActivity.startService(intent);
     }
 
-    private void establishVpn(String[] addresses, String[] routes, String[] excludedRoutes,
+    private void establishVpn(String[] addresses, String[] routes, String allowedApplications,
-            String allowedApplications, String disallowedApplications,
+            String disallowedApplications, @Nullable ProxyInfo proxyInfo,
-            @Nullable ProxyInfo proxyInfo, @Nullable ArrayList<Network> underlyingNetworks,
+            @Nullable ArrayList<Network> underlyingNetworks, boolean isAlwaysMetered)
-            boolean isAlwaysMetered, boolean addRoutesByIpPrefix)
             throws Exception {
         final Intent intent = new Intent(mActivity, MyVpnService.class)
                 .putExtra(mPackageName + ".cmd", MyVpnService.CMD_CONNECT)
                 .putExtra(mPackageName + ".addresses", TextUtils.join(",", addresses))
                 .putExtra(mPackageName + ".routes", TextUtils.join(",", routes))
-                .putExtra(mPackageName + ".excludedRoutes", TextUtils.join(",", excludedRoutes))
                 .putExtra(mPackageName + ".allowedapplications", allowedApplications)
                 .putExtra(mPackageName + ".disallowedapplications", disallowedApplications)
                 .putExtra(mPackageName + ".httpProxy", proxyInfo)
                 .putParcelableArrayListExtra(
                         mPackageName + ".underlyingNetworks", underlyingNetworks)
-                .putExtra(mPackageName + ".isAlwaysMetered", isAlwaysMetered)
+                .putExtra(mPackageName + ".isAlwaysMetered", isAlwaysMetered);
-                .putExtra(mPackageName + ".addRoutesByIpPrefix", addRoutesByIpPrefix);
         mActivity.startService(intent);
     }
 
     // TODO: Consider replacing arguments with a Builder.
     private void startVpn(
-            String[] addresses, String[] routes, String allowedApplications,
+        String[] addresses, String[] routes, String allowedApplications,
-            String disallowedApplications, @Nullable ProxyInfo proxyInfo,
+        String disallowedApplications, @Nullable ProxyInfo proxyInfo,
-            @Nullable ArrayList<Network> underlyingNetworks, boolean isAlwaysMetered)
+        @Nullable ArrayList<Network> underlyingNetworks, boolean isAlwaysMetered) throws Exception {
-            throws Exception {
-        startVpn(addresses, routes, new String[0] /* excludedRoutes */, allowedApplications,
-                disallowedApplications, proxyInfo, underlyingNetworks, isAlwaysMetered);
-    }
-
-    private void startVpn(
-            String[] addresses, String[] routes, String[] excludedRoutes,
-            String allowedApplications, String disallowedApplications,
-            @Nullable ProxyInfo proxyInfo,
-            @Nullable ArrayList<Network> underlyingNetworks, boolean isAlwaysMetered)
-            throws Exception {
-        startVpn(addresses, routes, new String[0] /* excludedRoutes */, allowedApplications,
-                disallowedApplications, proxyInfo, underlyingNetworks, isAlwaysMetered,
-                false /* addRoutesByIpPrefix */);
-    }
-
-    private void startVpn(
-            String[] addresses, String[] routes, String[] excludedRoutes,
-            String allowedApplications, String disallowedApplications,
-            @Nullable ProxyInfo proxyInfo,
-            @Nullable ArrayList<Network> underlyingNetworks, boolean isAlwaysMetered,
-            boolean addRoutesByIpPrefix)
-            throws Exception {
         prepareVpn();
 
         // Register a callback so we will be notified when our VPN comes up.
@@ -366,8 +340,8 @@ public class VpnTest {
         mCM.registerNetworkCallback(request, mCallback);  // Unregistered in tearDown.
 
         // Start the service and wait up for TIMEOUT_MS ms for the VPN to come up.
-        establishVpn(addresses, routes, excludedRoutes, allowedApplications, disallowedApplications,
+        establishVpn(addresses, routes, allowedApplications, disallowedApplications, proxyInfo,
-                proxyInfo, underlyingNetworks, isAlwaysMetered, addRoutesByIpPrefix);
+                underlyingNetworks, isAlwaysMetered);
         synchronized (mLock) {
             if (mNetwork == null) {
                  Log.i(TAG, "bf mLock");
@@ -587,12 +561,6 @@ public class VpnTest {
     }
 
     private void checkUdpEcho(String to, String expectedFrom) throws IOException {
-        checkUdpEcho(to, expectedFrom, expectedFrom != null);
-    }
-
-    private void checkUdpEcho(String to, String expectedFrom,
-            boolean expectConnectionOwnerIsVisible)
-            throws IOException {
         DatagramSocket s;
         InetAddress address = InetAddress.getByName(to);
         if (address instanceof Inet6Address) {  // http://b/18094870
@@ -616,7 +584,7 @@ public class VpnTest {
         try {
             if (expectedFrom != null) {
                 s.send(p);
-                checkConnectionOwnerUidUdp(s, expectConnectionOwnerIsVisible);
+                checkConnectionOwnerUidUdp(s, true);
                 s.receive(p);
                 MoreAsserts.assertEquals(data, p.getData());
             } else {
@@ -625,7 +593,7 @@ public class VpnTest {
                     s.receive(p);
                     fail("Received unexpected reply");
                 } catch (IOException expected) {
-                    checkConnectionOwnerUidUdp(s, expectConnectionOwnerIsVisible);
+                    checkConnectionOwnerUidUdp(s, false);
                 }
             }
         } finally {
@@ -633,38 +601,19 @@ public class VpnTest {
         }
     }
 
-    private void checkTrafficOnVpn(String destination) throws Exception {
-        final InetAddress address = InetAddress.getByName(destination);
-
-        if (address instanceof Inet6Address) {
-            checkUdpEcho(destination, "2001:db8:1:2::ffe");
-            checkTcpReflection(destination, "2001:db8:1:2::ffe");
-            checkPing(destination);
-        } else {
-            checkUdpEcho(destination, "192.0.2.2");
-            checkTcpReflection(destination, "192.0.2.2");
-        }
-
-    }
-
-    private void checkNoTrafficOnVpn(String destination) throws IOException {
-        checkUdpEcho(destination, null);
-        checkTcpReflection(destination, null);
-    }
-
     private void checkTrafficOnVpn() throws Exception {
-        checkTrafficOnVpn("192.0.2.251");
+        checkUdpEcho("192.0.2.251", "192.0.2.2");
-        checkTrafficOnVpn("2001:db8:dead:beef::f00");
+        checkUdpEcho("2001:db8:dead:beef::f00", "2001:db8:1:2::ffe");
+        checkPing("2001:db8:dead:beef::f00");
+        checkTcpReflection("192.0.2.252", "192.0.2.2");
+        checkTcpReflection("2001:db8:dead:beef::f00", "2001:db8:1:2::ffe");
     }
 
     private void checkNoTrafficOnVpn() throws Exception {
-        checkNoTrafficOnVpn("192.0.2.251");
+        checkUdpEcho("192.0.2.251", null);
-        checkNoTrafficOnVpn("2001:db8:dead:beef::f00");
+        checkUdpEcho("2001:db8:dead:beef::f00", null);
-    }
+        checkTcpReflection("192.0.2.252", null);
-
+        checkTcpReflection("2001:db8:dead:beef::f00", null);
-    private void checkTrafficBypassesVpn(String destination) throws Exception {
-        checkUdpEcho(destination, null, true /* expectVpnOwnedConnection */);
-        checkTcpReflection(destination, null);
     }
 
     private FileDescriptor openSocketFd(String host, int port, int timeoutMs) throws Exception {
@@ -1025,9 +974,9 @@ public class VpnTest {
         }
         Log.i(TAG, "Append shell app to disallowedApps: " + disallowedApps);
         startVpn(new String[] {"192.0.2.2/32", "2001:db8:1:2::ffe/128"},
-                new String[] {"192.0.2.0/24", "2001:db8::/32"},
+                 new String[] {"192.0.2.0/24", "2001:db8::/32"},
-                "", disallowedApps, null, null /* underlyingNetworks */,
+                 "", disallowedApps, null, null /* underlyingNetworks */,
-                false /* isAlwaysMetered */);
+                 false /* isAlwaysMetered */);
 
         assertSocketStillOpen(localFd, TEST_HOST);
         assertSocketStillOpen(remoteFd, TEST_HOST);
@@ -1039,74 +988,6 @@ public class VpnTest {
         assertFalse(nc.hasTransport(TRANSPORT_VPN));
     }
 
-    @Test
-    public void testExcludedRoutes() throws Exception {
-        if (!supportedHardware()) return;
-        if (!SdkLevel.isAtLeastT()) return;
-
-        // Shell app must not be put in here or it would kill the ADB-over-network use case
-        String allowedApps = mRemoteSocketFactoryClient.getPackageName() + "," + mPackageName;
-        startVpn(new String[]{"192.0.2.2/32", "2001:db8:1:2::ffe/128"} /* addresses */,
-                new String[]{"0.0.0.0/0", "::/0"} /* routes */,
-                new String[]{"192.0.2.0/24", "2001:db8::/32"} /* excludedRoutes */,
-                allowedApps, "" /* disallowedApplications */, null /* proxyInfo */,
-                null /* underlyingNetworks */, false /* isAlwaysMetered */);
-
-        // Excluded routes should bypass VPN.
-        checkTrafficBypassesVpn("192.0.2.1");
-        checkTrafficBypassesVpn("2001:db8:dead:beef::f00");
-        // Other routes should go through VPN, since default routes are included.
-        checkTrafficOnVpn("198.51.100.1");
-        checkTrafficOnVpn("2002:db8::1");
-    }
-
-    @Test
-    public void testIncludedRoutes() throws Exception {
-        if (!supportedHardware()) return;
-
-        // Shell app must not be put in here or it would kill the ADB-over-network use case
-        String allowedApps = mRemoteSocketFactoryClient.getPackageName() + "," + mPackageName;
-        startVpn(new String[]{"192.0.2.2/32", "2001:db8:1:2::ffe/128"} /* addresses */,
-                new String[]{"192.0.2.0/24", "2001:db8::/32"} /* routes */,
-                allowedApps, "" /* disallowedApplications */, null /* proxyInfo */,
-                null /* underlyingNetworks */, false /* isAlwaysMetered */);
-
-        // Included routes should go through VPN.
-        checkTrafficOnVpn("192.0.2.1");
-        checkTrafficOnVpn("2001:db8:dead:beef::f00");
-        // Other routes should bypass VPN, since default routes are not included.
-        checkTrafficBypassesVpn("198.51.100.1");
-        checkTrafficBypassesVpn("2002:db8::1");
-    }
-
-    @Test
-    public void testInterleavedRoutes() throws Exception {
-        if (!supportedHardware()) return;
-        if (!SdkLevel.isAtLeastT()) return;
-
-        // Shell app must not be put in here or it would kill the ADB-over-network use case
-        String allowedApps = mRemoteSocketFactoryClient.getPackageName() + "," + mPackageName;
-        startVpn(new String[]{"192.0.2.2/32", "2001:db8:1:2::ffe/128"} /* addresses */,
-                new String[]{"0.0.0.0/0", "192.0.2.0/32", "::/0", "2001:db8::/128"} /* routes */,
-                new String[]{"192.0.2.0/24", "2001:db8::/32"} /* excludedRoutes */,
-                allowedApps, "" /* disallowedApplications */, null /* proxyInfo */,
-                null /* underlyingNetworks */, false /* isAlwaysMetered */,
-                true /* addRoutesByIpPrefix */);
-
-        // Excluded routes should bypass VPN.
-        checkTrafficBypassesVpn("192.0.2.1");
-        checkTrafficBypassesVpn("2001:db8:dead:beef::f00");
-
-        // Included routes inside excluded routes should go through VPN, since the longest common
-        // prefix precedes.
-        checkTrafficOnVpn("192.0.2.0");
-        checkTrafficOnVpn("2001:db8::");
-
-        // Other routes should go through VPN, since default routes are included.
-        checkTrafficOnVpn("198.51.100.1");
-        checkTrafficOnVpn("2002:db8::1");
-    }
-
     @Test
     public void testGetConnectionOwnerUidSecurity() throws Exception {
         if (!supportedHardware()) return;

tests/cts/hostside/src/com/android/cts/net/HostsideNetworkTestCase.java

@@ -20,7 +20,6 @@ import com.android.compatibility.common.tradefed.build.CompatibilityBuildHelper;
 import com.android.ddmlib.Log;
 import com.android.ddmlib.testrunner.RemoteAndroidTestRunner;
 import com.android.ddmlib.testrunner.TestResult.TestStatus;
-import com.android.modules.utils.build.testing.DeviceSdkLevel;
 import com.android.tradefed.build.IBuildInfo;
 import com.android.tradefed.device.DeviceNotAvailableException;
 import com.android.tradefed.result.CollectingTestListener;
@@ -43,7 +42,6 @@ abstract class HostsideNetworkTestCase extends DeviceTestCase implements IAbiRec
     protected static final String TAG = "HostsideNetworkTests";
     protected static final String TEST_PKG = "com.android.cts.net.hostside";
     protected static final String TEST_APK = "CtsHostsideNetworkTestsApp.apk";
-    protected static final String TEST_APK_NEXT = "CtsHostsideNetworkTestsAppNext.apk";
     protected static final String TEST_APP2_PKG = "com.android.cts.net.hostside.app2";
     protected static final String TEST_APP2_APK = "CtsHostsideNetworkTestsApp2.apk";
 
@@ -67,12 +65,8 @@ abstract class HostsideNetworkTestCase extends DeviceTestCase implements IAbiRec
         assertNotNull(mAbi);
         assertNotNull(mCtsBuild);
 
-        DeviceSdkLevel deviceSdkLevel = new DeviceSdkLevel(getDevice());
-        String testApk = deviceSdkLevel.isDeviceAtLeastT() ? TEST_APK_NEXT
-                : TEST_APK;
-
         uninstallPackage(TEST_PKG, false);
-        installPackage(testApk);
+        installPackage(TEST_APK);
     }
 
     @Override

tests/cts/hostside/src/com/android/cts/net/HostsideVpnTests.java

@@ -104,16 +104,4 @@ public class HostsideVpnTests extends HostsideNetworkTestCase {
         runDeviceTests(TEST_PKG, TEST_PKG + ".VpnTest",
                 "testDownloadWithDownloadManagerDisallowed");
     }
-
-    public void testExcludedRoutes() throws Exception {
-        runDeviceTests(TEST_PKG, TEST_PKG + ".VpnTest", "testExcludedRoutes");
-    }
-
-    public void testIncludedRoutes() throws Exception {
-        runDeviceTests(TEST_PKG, TEST_PKG + ".VpnTest", "testIncludedRoutes");
-    }
-
-    public void testInterleavedRoutes() throws Exception {
-        runDeviceTests(TEST_PKG, TEST_PKG + ".VpnTest", "testInterleavedRoutes");
-    }
 }