diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index c9bdcf1a73b5f..2fda08e2a5759 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -151,6 +151,13 @@ public class Vpn { .multiply(BigInteger.valueOf(howManyPercentIsMost)) .divide(BigInteger.valueOf(100)); } + // How many routes to evaluate before bailing and declaring this Vpn should provide + // the INTERNET capability. This is necessary because computing the adress space is + // O(n²) and this is running in the system service, so a limit is needed to alleviate + // the risk of attack. + // This is taken as a total of IPv4 + IPV6 routes for simplicity, but the algorithm + // is actually O(n²)+O(n²). + private static final int MAX_ROUTES_TO_EVALUATE = 150; // TODO: create separate trackers for each unique VPN to support // automated reconnection @@ -862,10 +869,12 @@ public class Vpn { */ @VisibleForTesting static boolean providesRoutesToMostDestinations(LinkProperties lp) { + final List routes = lp.getAllRoutes(); + if (routes.size() > MAX_ROUTES_TO_EVALUATE) return true; final Comparator prefixLengthComparator = IpPrefix.lengthComparator(); TreeSet ipv4Prefixes = new TreeSet<>(prefixLengthComparator); TreeSet ipv6Prefixes = new TreeSet<>(prefixLengthComparator); - for (final RouteInfo route : lp.getAllRoutes()) { + for (final RouteInfo route : routes) { IpPrefix destination = route.getDestination(); if (destination.isIPv4()) { ipv4Prefixes.add(destination); diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index f59850d45ae32..e377a472530e2 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -70,6 +70,7 @@ import android.os.Build.VERSION_CODES; import android.os.Bundle; import android.os.INetworkManagementService; import android.os.Looper; +import android.os.SystemClock; import android.os.UserHandle; import android.os.UserManager; import android.support.test.filters.SmallTest; @@ -88,6 +89,8 @@ import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.net.Inet4Address; +import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -639,4 +642,32 @@ public class VpnTest { lp.addRoute(new RouteInfo(new IpPrefix("::/1"))); assertTrue(Vpn.providesRoutesToMostDestinations(lp)); } + + @Test + public void testDoesNotLockUpWithTooManyRoutes() { + final LinkProperties lp = new LinkProperties(); + final byte[] ad = new byte[4]; + // Actually evaluating this many routes under 1500ms is impossible on + // current hardware and for some time, as the algorithm is O(n²). + // Make sure the system has a safeguard against this and does not + // lock up. + final int MAX_ROUTES = 4000; + final long MAX_ALLOWED_TIME_MS = 1500; + for (int i = 0; i < MAX_ROUTES; ++i) { + ad[0] = (byte)((i >> 24) & 0xFF); + ad[1] = (byte)((i >> 16) & 0xFF); + ad[2] = (byte)((i >> 8) & 0xFF); + ad[3] = (byte)(i & 0xFF); + try { + lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.getByAddress(ad), 32))); + } catch (UnknownHostException e) { + // UnknownHostException is only thrown for an address of illegal length, + // which can't happen in the case above. + } + } + final long start = SystemClock.currentThreadTimeMillis(); + assertTrue(Vpn.providesRoutesToMostDestinations(lp)); + final long end = SystemClock.currentThreadTimeMillis(); + assertTrue(end - start < MAX_ALLOWED_TIME_MS); + } }