From d9ba74953bf3e1a4298e3d0fa9214f65d5b561a7 Mon Sep 17 00:00:00 2001 From: Taras Antoshchuk Date: Fri, 7 Jan 2022 18:48:25 +0000 Subject: [PATCH] Revert "Revert^2 "Revert "Add CTS tests for exclude VPN routes A..." Revert "Revert "Revert "Revert "Add APIs that allow to exclude r..." Revert "Revert "Revert "Revert "Add VpnServiceBuilderShim for Vp..." Revert submission 1941195-revert-1931760-reland-vpn-impl-part-2-WGARECSJEM Reason for revert: fixed merge conflict with aosp/1938197 Reverted Changes: Ic25e5e4ea:Revert^2 "Revert "Add CTS tests for exclude VPN ro... Ic72cafcf5:Revert "Revert "Revert "Add APIs that allow to exc... I53802190a:Revert "Revert "Revert "Add VpnServiceBuilderShim ... Change-Id: I5a6ceaae21c563de8bb71338c29973f58178b456 --- tests/cts/hostside/Android.bp | 3 + tests/cts/hostside/app/Android.bp | 33 +++- .../cts/net/hostside/MyVpnService.java | 93 ++++++---- .../com/android/cts/net/hostside/VpnTest.java | 165 +++++++++++++++--- .../cts/net/HostsideNetworkTestCase.java | 8 +- .../com/android/cts/net/HostsideVpnTests.java | 12 ++ 6 files changed, 254 insertions(+), 60 deletions(-) diff --git a/tests/cts/hostside/Android.bp b/tests/cts/hostside/Android.bp index f72a458414..b68406871b 100644 --- a/tests/cts/hostside/Android.bp +++ b/tests/cts/hostside/Android.bp @@ -25,6 +25,9 @@ java_test_host { "cts-tradefed", "tradefed", ], + static_libs: [ + "modules-utils-build-testing", + ], // Tag this module as a cts test artifact test_suites: [ "cts", diff --git a/tests/cts/hostside/app/Android.bp b/tests/cts/hostside/app/Android.bp index 63572c3598..12e7d33eea 100644 --- a/tests/cts/hostside/app/Android.bp +++ b/tests/cts/hostside/app/Android.bp @@ -18,12 +18,8 @@ package { default_applicable_licenses: ["Android-Apache-2.0"], } -android_test_helper_app { - name: "CtsHostsideNetworkTestsApp", - defaults: [ - "cts_support_defaults", - "framework-connectivity-test-defaults", - ], +java_defaults { + name: "CtsHostsideNetworkTestsAppDefaults", platform_apis: true, static_libs: [ "CtsHostsideNetworkTestsAidl", @@ -48,3 +44,28 @@ android_test_helper_app { "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", + ], +} diff --git a/tests/cts/hostside/app/src/com/android/cts/net/hostside/MyVpnService.java b/tests/cts/hostside/app/src/com/android/cts/net/hostside/MyVpnService.java index 855abf7f24..449454ef09 100644 --- a/tests/cts/hostside/app/src/com/android/cts/net/hostside/MyVpnService.java +++ b/tests/cts/hostside/app/src/com/android/cts/net/hostside/MyVpnService.java @@ -18,17 +18,26 @@ 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 { @@ -67,39 +76,62 @@ public class MyVpnService extends VpnService { (underlyingNetworks != null) ? underlyingNetworks.toArray(new Network[0]) : null); } - private void start(String packageName, Intent intent) { - Builder builder = new Builder(); + private String parseIpAndMaskListArgument(String packageName, Intent intent, String argName, + BiConsumer consumer) { + final String addresses = intent.getStringExtra(packageName + "." + argName); - String addresses = intent.getStringExtra(packageName + ".addresses"); - if (addresses != null) { - String[] addressArray = addresses.split(","); - for (int i = 0; i < addressArray.length; i++) { - String[] prefixAndMask = addressArray[i].split("/"); - try { - InetAddress address = InetAddress.getByName(prefixAndMask[0]); - int prefixLength = Integer.parseInt(prefixAndMask[1]); - builder.addAddress(address, prefixLength); - } catch (UnknownHostException|NumberFormatException| - ArrayIndexOutOfBoundsException e) { - continue; - } - } + if (TextUtils.isEmpty(addresses)) { + return null; } - String routes = intent.getStringExtra(packageName + ".routes"); - if (routes != null) { - String[] routeArray = routes.split(","); - for (int i = 0; i < routeArray.length; i++) { - String[] prefixAndMask = routeArray[i].split("/"); + final String[] addressesArray = addresses.split(","); + for (String address : addressesArray) { + final Pair ipAndMask = NetworkUtils.parseIpAndMask(address); + consumer.accept(ipAndMask.first, ipAndMask.second); + } + + return addresses; + } + + private String parseIpPrefixListArgument(String packageName, Intent intent, String argName, + Consumer 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", + builder::addAddress); + + String addedRoutes; + if (SdkLevel.isAtLeastT() && intent.getBooleanExtra(packageName + ".addRoutesByIpPrefix", + false)) { + addedRoutes = parseIpPrefixListArgument(packageName, intent, "routes", (prefix) -> { try { - InetAddress address = InetAddress.getByName(prefixAndMask[0]); - int prefixLength = Integer.parseInt(prefixAndMask[1]); - builder.addRoute(address, prefixLength); - } catch (UnknownHostException|NumberFormatException| - ArrayIndexOutOfBoundsException e) { - continue; + vpnServiceBuilderShim.addRoute(builder, prefix); + } catch (UnsupportedApiLevelException e) { + throw new RuntimeException(e); } - } + }); + } else { + addedRoutes = parseIpAndMaskListArgument(packageName, intent, "routes", + builder::addRoute); + } + + String excludedRoutes = null; + if (SdkLevel.isAtLeastT()) { + excludedRoutes = parseIpPrefixListArgument(packageName, intent, "excludedRoutes", + (prefix) -> { + try { + vpnServiceBuilderShim.excludeRoute(builder, prefix); + } catch (UnsupportedApiLevelException e) { + throw new RuntimeException(e); + } + }); } String allowed = intent.getStringExtra(packageName + ".allowedapplications"); @@ -152,7 +184,8 @@ public class MyVpnService extends VpnService { Log.i(TAG, "Establishing VPN," + " addresses=" + addresses - + " routes=" + routes + + " addedRoutes=" + addedRoutes + + " excludedRoutes=" + excludedRoutes + " allowedApplications=" + allowed + " disallowedApplications=" + disallowed); diff --git a/tests/cts/hostside/app/src/com/android/cts/net/hostside/VpnTest.java b/tests/cts/hostside/app/src/com/android/cts/net/hostside/VpnTest.java index 65ea4b1fe3..708d600fff 100755 --- a/tests/cts/hostside/app/src/com/android/cts/net/hostside/VpnTest.java +++ b/tests/cts/hostside/app/src/com/android/cts/net/hostside/VpnTest.java @@ -298,28 +298,54 @@ public class VpnTest { mActivity.startService(intent); } - private void establishVpn(String[] addresses, String[] routes, String allowedApplications, - String disallowedApplications, @Nullable ProxyInfo proxyInfo, - @Nullable ArrayList underlyingNetworks, boolean isAlwaysMetered) + private void establishVpn(String[] addresses, String[] routes, String[] excludedRoutes, + String allowedApplications, String disallowedApplications, + @Nullable ProxyInfo proxyInfo, @Nullable ArrayList underlyingNetworks, + 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 disallowedApplications, @Nullable ProxyInfo proxyInfo, - @Nullable ArrayList underlyingNetworks, boolean isAlwaysMetered) throws Exception { + String[] addresses, String[] routes, String allowedApplications, + String disallowedApplications, @Nullable ProxyInfo proxyInfo, + @Nullable ArrayList underlyingNetworks, boolean isAlwaysMetered) + 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 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 underlyingNetworks, boolean isAlwaysMetered, + boolean addRoutesByIpPrefix) + throws Exception { prepareVpn(); // Register a callback so we will be notified when our VPN comes up. @@ -340,8 +366,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, allowedApplications, disallowedApplications, proxyInfo, - underlyingNetworks, isAlwaysMetered); + establishVpn(addresses, routes, excludedRoutes, allowedApplications, disallowedApplications, + proxyInfo, underlyingNetworks, isAlwaysMetered, addRoutesByIpPrefix); synchronized (mLock) { if (mNetwork == null) { Log.i(TAG, "bf mLock"); @@ -561,6 +587,12 @@ 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 @@ -584,7 +616,7 @@ public class VpnTest { try { if (expectedFrom != null) { s.send(p); - checkConnectionOwnerUidUdp(s, true); + checkConnectionOwnerUidUdp(s, expectConnectionOwnerIsVisible); s.receive(p); MoreAsserts.assertEquals(data, p.getData()); } else { @@ -593,7 +625,7 @@ public class VpnTest { s.receive(p); fail("Received unexpected reply"); } catch (IOException expected) { - checkConnectionOwnerUidUdp(s, false); + checkConnectionOwnerUidUdp(s, expectConnectionOwnerIsVisible); } } } finally { @@ -601,19 +633,38 @@ 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 { - checkUdpEcho("192.0.2.251", "192.0.2.2"); - 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"); + checkTrafficOnVpn("192.0.2.251"); + checkTrafficOnVpn("2001:db8:dead:beef::f00"); } private void checkNoTrafficOnVpn() throws Exception { - checkUdpEcho("192.0.2.251", null); - checkUdpEcho("2001:db8:dead:beef::f00", null); - checkTcpReflection("192.0.2.252", null); - checkTcpReflection("2001:db8:dead:beef::f00", null); + checkNoTrafficOnVpn("192.0.2.251"); + checkNoTrafficOnVpn("2001:db8:dead:beef::f00"); + } + + 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 { @@ -974,9 +1025,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"}, - "", disallowedApps, null, null /* underlyingNetworks */, - false /* isAlwaysMetered */); + new String[] {"192.0.2.0/24", "2001:db8::/32"}, + "", disallowedApps, null, null /* underlyingNetworks */, + false /* isAlwaysMetered */); assertSocketStillOpen(localFd, TEST_HOST); assertSocketStillOpen(remoteFd, TEST_HOST); @@ -988,6 +1039,74 @@ 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; diff --git a/tests/cts/hostside/src/com/android/cts/net/HostsideNetworkTestCase.java b/tests/cts/hostside/src/com/android/cts/net/HostsideNetworkTestCase.java index 89c79d3677..cc07fd1089 100644 --- a/tests/cts/hostside/src/com/android/cts/net/HostsideNetworkTestCase.java +++ b/tests/cts/hostside/src/com/android/cts/net/HostsideNetworkTestCase.java @@ -20,6 +20,7 @@ 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; @@ -42,6 +43,7 @@ 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"; @@ -65,8 +67,12 @@ 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(TEST_APK); + installPackage(testApk); } @Override diff --git a/tests/cts/hostside/src/com/android/cts/net/HostsideVpnTests.java b/tests/cts/hostside/src/com/android/cts/net/HostsideVpnTests.java index 180015d433..3821f87a0e 100644 --- a/tests/cts/hostside/src/com/android/cts/net/HostsideVpnTests.java +++ b/tests/cts/hostside/src/com/android/cts/net/HostsideVpnTests.java @@ -104,4 +104,16 @@ 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"); + } }