From 7153b105e78121a84c962c59f25b6a399339b253 Mon Sep 17 00:00:00 2001 From: Matt Buckley Date: Fri, 7 Jan 2022 17:44:52 +0000 Subject: [PATCH] Revert "Revert "Revert "Add APIs that allow to exclude routes fr..." Revert "Revert "Revert "Add VpnServiceBuilderShim for VpnService..." Revert submission 1931760-reland-vpn-impl-part-2 Reason for revert: DroidMonitor-triggered revert due to breakage https://android-build.googleplex.com/builds/quarterdeck?branch=aosp-master&target=test_suites_x86_64&lkgb=8053795&lkbb=8053908&fkbb=8053828, bug b/213588956. BUG: b/213588956 Reverted Changes: Ic8ed8fce7:Revert "Revert "Add CTS tests for exclude VPN rout... I07104340a:Revert "Revert "Add VpnServiceBuilderShim for VpnS... I7c69b7244:Revert "Revert "Add APIs that allow to exclude rou... Change-Id: Ic72cafcf5c7e2c62236f1cef61b0764882bacaad --- core/api/current.txt | 2 - core/java/android/net/VpnService.java | 123 +++--------------- .../com/android/internal/net/VpnConfig.java | 40 ++---- .../com/android/server/connectivity/Vpn.java | 7 +- 4 files changed, 31 insertions(+), 141 deletions(-) diff --git a/core/api/current.txt b/core/api/current.txt index 97328e846e334..b42d6aaf9386c 100644 --- a/core/api/current.txt +++ b/core/api/current.txt @@ -26895,13 +26895,11 @@ package android.net { method @NonNull public android.net.VpnService.Builder addDnsServer(@NonNull java.net.InetAddress); method @NonNull public android.net.VpnService.Builder addDnsServer(@NonNull String); method @NonNull public android.net.VpnService.Builder addRoute(@NonNull java.net.InetAddress, int); - method @NonNull public android.net.VpnService.Builder addRoute(@NonNull android.net.IpPrefix); method @NonNull public android.net.VpnService.Builder addRoute(@NonNull String, int); method @NonNull public android.net.VpnService.Builder addSearchDomain(@NonNull String); method @NonNull public android.net.VpnService.Builder allowBypass(); method @NonNull public android.net.VpnService.Builder allowFamily(int); method @Nullable public android.os.ParcelFileDescriptor establish(); - method @NonNull public android.net.VpnService.Builder excludeRoute(@NonNull android.net.IpPrefix); method @NonNull public android.net.VpnService.Builder setBlocking(boolean); method @NonNull public android.net.VpnService.Builder setConfigureIntent(@NonNull android.app.PendingIntent); method @NonNull public android.net.VpnService.Builder setHttpProxy(@NonNull android.net.ProxyInfo); diff --git a/core/java/android/net/VpnService.java b/core/java/android/net/VpnService.java index 1ae1b050d32fb..2ced056937556 100644 --- a/core/java/android/net/VpnService.java +++ b/core/java/android/net/VpnService.java @@ -41,7 +41,6 @@ import android.os.RemoteException; import android.os.ServiceManager; import android.os.UserHandle; -import com.android.internal.annotations.VisibleForTesting; import com.android.internal.net.NetworkUtilsInternal; import com.android.internal.net.VpnConfig; @@ -51,7 +50,6 @@ import java.net.Inet6Address; import java.net.InetAddress; import java.net.Socket; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Set; @@ -473,13 +471,6 @@ public class VpnService extends Service { } } - private static void checkNonPrefixBytes(@NonNull InetAddress address, int prefixLength) { - final IpPrefix prefix = new IpPrefix(address, prefixLength); - if (!prefix.getAddress().equals(address)) { - throw new IllegalArgumentException("Bad address"); - } - } - /** * Helper class to create a VPN interface. This class should be always * used within the scope of the outer {@link VpnService}. @@ -490,9 +481,9 @@ public class VpnService extends Service { private final VpnConfig mConfig = new VpnConfig(); @UnsupportedAppUsage - private final List mAddresses = new ArrayList<>(); + private final List mAddresses = new ArrayList(); @UnsupportedAppUsage - private final List mRoutes = new ArrayList<>(); + private final List mRoutes = new ArrayList(); public Builder() { mConfig.user = VpnService.this.getClass().getName(); @@ -564,6 +555,7 @@ public class VpnService extends Service { throw new IllegalArgumentException("Bad address"); } mAddresses.add(new LinkAddress(address, prefixLength)); + mConfig.updateAllowedFamilies(address); return this; } @@ -583,32 +575,6 @@ public class VpnService extends Service { return addAddress(InetAddress.parseNumericAddress(address), prefixLength); } - /** - * Add a network route to the VPN interface. Both IPv4 and IPv6 - * routes are supported. - * - * If a route with the same destination is already present, its type will be updated. - * - * @throws IllegalArgumentException if the route is invalid. - */ - @NonNull - private Builder addRoute(@NonNull IpPrefix prefix, int type) { - check(prefix.getAddress(), prefix.getPrefixLength()); - - final RouteInfo newRoute = new RouteInfo(prefix, /* gateway */ - null, /* interface */ null, type); - - final int index = findRouteIndexByDestination(newRoute); - - if (index == -1) { - mRoutes.add(newRoute); - } else { - mRoutes.set(index, newRoute); - } - - return this; - } - /** * Add a network route to the VPN interface. Both IPv4 and IPv6 * routes are supported. @@ -616,39 +582,25 @@ public class VpnService extends Service { * Adding a route implicitly allows traffic from that address family * (i.e., IPv4 or IPv6) to be routed over the VPN. @see #allowFamily * - * Calling this method overrides previous calls to {@link #excludeRoute} for the same - * destination. - * - * If multiple routes match the packet destination, route with the longest prefix takes - * precedence. - * * @throws IllegalArgumentException if the route is invalid. */ @NonNull public Builder addRoute(@NonNull InetAddress address, int prefixLength) { - checkNonPrefixBytes(address, prefixLength); + check(address, prefixLength); - return addRoute(new IpPrefix(address, prefixLength), RouteInfo.RTN_UNICAST); - } - - /** - * Add a network route to the VPN interface. Both IPv4 and IPv6 - * routes are supported. - * - * Adding a route implicitly allows traffic from that address family - * (i.e., IPv4 or IPv6) to be routed over the VPN. @see #allowFamily - * - * Calling this method overrides previous calls to {@link #excludeRoute} for the same - * destination. - * - * If multiple routes match the packet destination, route with the longest prefix takes - * precedence. - * - * @throws IllegalArgumentException if the route is invalid. - */ - @NonNull - public Builder addRoute(@NonNull IpPrefix prefix) { - return addRoute(prefix, RouteInfo.RTN_UNICAST); + int offset = prefixLength / 8; + byte[] bytes = address.getAddress(); + if (offset < bytes.length) { + for (bytes[offset] <<= prefixLength % 8; offset < bytes.length; ++offset) { + if (bytes[offset] != 0) { + throw new IllegalArgumentException("Bad address"); + } + } + } + mRoutes.add(new RouteInfo(new IpPrefix(address, prefixLength), null, null, + RouteInfo.RTN_UNICAST)); + mConfig.updateAllowedFamilies(address); + return this; } /** @@ -659,12 +611,6 @@ public class VpnService extends Service { * Adding a route implicitly allows traffic from that address family * (i.e., IPv4 or IPv6) to be routed over the VPN. @see #allowFamily * - * Calling this method overrides previous calls to {@link #excludeRoute} for the same - * destination. - * - * If multiple routes match the packet destination, route with the longest prefix takes - * precedence. - * * @throws IllegalArgumentException if the route is invalid. * @see #addRoute(InetAddress, int) */ @@ -673,23 +619,6 @@ public class VpnService extends Service { return addRoute(InetAddress.parseNumericAddress(address), prefixLength); } - /** - * Exclude a network route from the VPN interface. Both IPv4 and IPv6 - * routes are supported. - * - * Calling this method overrides previous calls to {@link #addRoute} for the same - * destination. - * - * If multiple routes match the packet destination, route with the longest prefix takes - * precedence. - * - * @throws IllegalArgumentException if the route is invalid. - */ - @NonNull - public Builder excludeRoute(@NonNull IpPrefix prefix) { - return addRoute(prefix, RouteInfo.RTN_THROW); - } - /** * Add a DNS server to the VPN connection. Both IPv4 and IPv6 * addresses are supported. If none is set, the DNS servers of @@ -971,23 +900,5 @@ public class VpnService extends Service { throw new IllegalStateException(e); } } - - private int findRouteIndexByDestination(RouteInfo route) { - for (int i = 0; i < mRoutes.size(); i++) { - if (mRoutes.get(i).getDestination().equals(route.getDestination())) { - return i; - } - } - return -1; - } - - /** - * Method for testing, to observe mRoutes while builder is being used. - * @hide - */ - @VisibleForTesting - public List routes() { - return Collections.unmodifiableList(mRoutes); - } } } diff --git a/core/java/com/android/internal/net/VpnConfig.java b/core/java/com/android/internal/net/VpnConfig.java index 2ae56f8089727..2e7629a76deea 100644 --- a/core/java/com/android/internal/net/VpnConfig.java +++ b/core/java/com/android/internal/net/VpnConfig.java @@ -34,6 +34,8 @@ import android.os.Parcel; import android.os.Parcelable; import android.os.UserHandle; +import java.net.Inet4Address; +import java.net.InetAddress; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -91,8 +93,8 @@ public class VpnConfig implements Parcelable { public String interfaze; public String session; public int mtu = -1; - public List addresses = new ArrayList<>(); - public List routes = new ArrayList<>(); + public List addresses = new ArrayList(); + public List routes = new ArrayList(); public List dnsServers; public List searchDomains; public List allowedApplications; @@ -112,32 +114,12 @@ public class VpnConfig implements Parcelable { public VpnConfig() { } - public VpnConfig(VpnConfig other) { - user = other.user; - interfaze = other.interfaze; - session = other.session; - mtu = other.mtu; - addresses = copyOf(other.addresses); - routes = copyOf(other.routes); - dnsServers = copyOf(other.dnsServers); - searchDomains = copyOf(other.searchDomains); - allowedApplications = copyOf(other.allowedApplications); - disallowedApplications = copyOf(other.disallowedApplications); - configureIntent = other.configureIntent; - startTime = other.startTime; - legacy = other.legacy; - blocking = other.blocking; - allowBypass = other.allowBypass; - allowIPv4 = other.allowIPv4; - allowIPv6 = other.allowIPv6; - isMetered = other.isMetered; - underlyingNetworks = other.underlyingNetworks != null ? Arrays.copyOf( - other.underlyingNetworks, other.underlyingNetworks.length) : null; - proxyInfo = other.proxyInfo; - } - - private static List copyOf(List list) { - return list != null ? new ArrayList<>(list) : null; + public void updateAllowedFamilies(InetAddress address) { + if (address instanceof Inet4Address) { + allowIPv4 = true; + } else { + allowIPv6 = true; + } } public void addLegacyRoutes(String routesStr) { @@ -149,6 +131,7 @@ public class VpnConfig implements Parcelable { //each route is ip/prefix RouteInfo info = new RouteInfo(new IpPrefix(route), null, null, RouteInfo.RTN_UNICAST); this.routes.add(info); + updateAllowedFamilies(info.getDestination().getAddress()); } } @@ -161,6 +144,7 @@ public class VpnConfig implements Parcelable { //each address is ip/prefix LinkAddress addr = new LinkAddress(address); this.addresses.add(addr); + updateAllowedFamilies(addr.getAddress()); } } diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index 9a9c3ea05d190..2c666b5bb7c7d 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -1208,11 +1208,8 @@ public class Vpn { for (RouteInfo route : mConfig.routes) { lp.addRoute(route); InetAddress address = route.getDestination().getAddress(); - - if (route.getType() == RouteInfo.RTN_UNICAST) { - allowIPv4 |= address instanceof Inet4Address; - allowIPv6 |= address instanceof Inet6Address; - } + allowIPv4 |= address instanceof Inet4Address; + allowIPv6 |= address instanceof Inet6Address; } }