From 275ee60963c2c69f865105b23ca07b5db49ffd28 Mon Sep 17 00:00:00 2001
From: Lorenzo Colitti <lorenzo@google.com>
Date: Tue, 9 Aug 2022 19:29:12 +0900
Subject: [PATCH] Never satisfy requests with networks that aren't connected.

Currently, NetworkAgentInfo#satisfies determines whether a
network exists (and should satisfy requests) based on
|created|, not |everConnected|. This is theoretically incorrect
because networks in CONNECTING state or similar must never
satisfy requests or call callbacks.

This cannot happen for requests. New requests are only added to
networks by NetworkAgentInfo#addRequest, which is only called by
computeNetworkReassignment via updateSatisfiersForRematchRequest,
which skips any network with everConnected = false. It can
potentially happen to listens though.

Also, this cannot ever happen for physical networks, because for
physical networks, created is set at the same time as
everConnected, the first time they enter CONNECTED state.

In theory, this can happen for VPNs since ag/988402 , which
changed the code to create VPN networks as soon as they enter
CONNECTING state. That CL added the everConnected boolean to
NetworkAgentInfo but did not update the satisfies and
satisfiesImmutableCapabilitiesOf checks to use everConnected.
In practice, even for VPNs this is very unlikely, since VPNs
advance from CONNECTING to CONNECTED essentially immediately.
Even if they didn't, sending callbacks before they enter
CONNECTED state would be incorrect because the app receiving
the callback would have no way of knowing that the network is
actually not yet connected.

With this change, some checks for everConnected become spurious,
because a network with everConnected=false cannot satisfy either
requests or listens. Remove these checks.

Test: extensively tested by existing ConnectivityServiceTest
Change-Id: I024bb2b85bc57228d53e69a7707c7ce7ac756259
---
 .../src/com/android/server/ConnectivityService.java    | 10 ++--------
 .../android/server/connectivity/NetworkAgentInfo.java  |  9 ++++-----
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index 1caca01256..fe14e5ff9a 100644
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -7495,9 +7495,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
             notifyIfacesChangedForNetworkStats();
             networkAgent.networkMonitor().notifyLinkPropertiesChanged(
                     new LinkProperties(newLp, true /* parcelSensitiveFields */));
-            if (networkAgent.everConnected) {
-                notifyNetworkCallbacks(networkAgent, ConnectivityManager.CALLBACK_IP_CHANGED);
-            }
+            notifyNetworkCallbacks(networkAgent, ConnectivityManager.CALLBACK_IP_CHANGED);
         }
 
         mKeepaliveTracker.handleCheckKeepalivesStillValid(networkAgent);
@@ -8749,9 +8747,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
         // Gather the list of all relevant agents.
         final ArrayList<NetworkAgentInfo> nais = new ArrayList<>();
         for (final NetworkAgentInfo nai : mNetworkAgentInfos) {
-            if (!nai.everConnected) {
-                continue;
-            }
             nais.add(nai);
         }
 
@@ -8875,7 +8870,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
 
         for (final NetworkAgentInfo nai : nais) {
-            if (!nai.everConnected) continue;
             final boolean oldBackground = oldBgNetworks.contains(nai);
             // Process listen requests and update capabilities if the background state has
             // changed for this network. For consistency with previous behavior, send onLost
@@ -9468,7 +9462,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
             }
         }
         for (NetworkAgentInfo nai : mNetworkAgentInfos) {
-            if (nai.everConnected && (activeNetIds.contains(nai.network().netId) || nai.isVPN())) {
+            if (activeNetIds.contains(nai.network().netId) || nai.isVPN()) {
                 defaultNetworks.add(nai.network);
             }
         }
diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java
index 04f378f2e0..88a5f9c44c 100644
--- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java
+++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java
@@ -983,14 +983,13 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable {
 
     // Does this network satisfy request?
     public boolean satisfies(NetworkRequest request) {
-        return created &&
-                request.networkCapabilities.satisfiedByNetworkCapabilities(networkCapabilities);
+        return everConnected
+                && request.networkCapabilities.satisfiedByNetworkCapabilities(networkCapabilities);
     }
 
     public boolean satisfiesImmutableCapabilitiesOf(NetworkRequest request) {
-        return created &&
-                request.networkCapabilities.satisfiedByImmutableNetworkCapabilities(
-                        networkCapabilities);
+        return everConnected && request.networkCapabilities.satisfiedByImmutableNetworkCapabilities(
+                networkCapabilities);
     }
 
     /** Whether this network is a VPN. */