Limit the number of routes for performance
In evaluating whether "most" of the addressing space is
covered, the list of routes are obtained from a third-party
app, so it's possbile the system service stalls unless
some limit is enforced on how much work it has to do.
This change limits the number of routes to 400, as determined
by time measurement on various devices.
Bug: 74176086
Test: runtest framework-net
Change-Id: Ie4a96098bc044ade87b188839586f14dd101c100
diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java
index c9bdcf1..2fda08e 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 @@
.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 @@
*/
@VisibleForTesting
static boolean providesRoutesToMostDestinations(LinkProperties lp) {
+ final List<RouteInfo> routes = lp.getAllRoutes();
+ if (routes.size() > MAX_ROUTES_TO_EVALUATE) return true;
final Comparator<IpPrefix> prefixLengthComparator = IpPrefix.lengthComparator();
TreeSet<IpPrefix> ipv4Prefixes = new TreeSet<>(prefixLengthComparator);
TreeSet<IpPrefix> 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 f59850d..e377a47 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.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.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 @@
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);
+ }
}