Enforce structural NetworkAgent constraints

Avoid network agents trying to do things that the stack
does not support.

Test: new test in this change
Change-Id: I3ffa1c3ddbb0b648b06862dc0a44f8525d358acc

service/src/com/android/server/ConnectivityService.java

@@ -4156,7 +4156,14 @@ public class ConnectivityService extends IConnectivityManager.Stub
 
             switch (msg.what) {
                 case NetworkAgent.EVENT_NETWORK_CAPABILITIES_CHANGED: {
-                    nai.setDeclaredCapabilities((NetworkCapabilities) arg.second);
+                    final NetworkCapabilities proposed = (NetworkCapabilities) arg.second;
+                    if (!nai.respectsNcStructuralConstraints(proposed)) {
+                        Log.wtf(TAG, "Agent " + nai + " violates nc structural constraints : "
+                                + nai.networkCapabilities + " -> " + proposed);
+                        disconnectAndDestroyNetwork(nai);
+                        return;
+                    }
+                    nai.setDeclaredCapabilities(proposed);
                     final NetworkCapabilities sanitized =
                             nai.getDeclaredCapabilitiesSanitized(mCarrierPrivilegeAuthenticator);
                     maybeUpdateWifiRoamTimestamp(nai, sanitized);

service/src/com/android/server/connectivity/NetworkAgentInfo.java

@@ -17,6 +17,7 @@
 package com.android.server.connectivity;
 
 import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport;
+import static android.net.NetworkCapabilities.NET_CAPABILITY_LOCAL_NETWORK;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED;
 import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR;
 import static android.net.NetworkCapabilities.TRANSPORT_ETHERNET;
@@ -427,13 +428,29 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable {
     // Indicate whether this device has the automotive feature.
     private final boolean mHasAutomotiveFeature;
 
+    /**
+     * Checks that a proposed update to the NCs of this NAI satisfies structural constraints.
+     *
+     * Some changes to NetworkCapabilities are structurally not supported by the stack, and
+     * NetworkAgents are absolutely never allowed to try and do them. When one of these is
+     * violated, this method returns false, which has ConnectivityService disconnect the network ;
+     * this is meant to guarantee that no implementor ever tries to do this.
+     */
+    public boolean respectsNcStructuralConstraints(@NonNull final NetworkCapabilities proposedNc) {
+        if (networkCapabilities.hasCapability(NET_CAPABILITY_LOCAL_NETWORK)
+                != proposedNc.hasCapability(NET_CAPABILITY_LOCAL_NETWORK)) {
+            return false;
+        }
+        return true;
+    }
+
     /**
      * Sets the capabilities sent by the agent for later retrieval.
-     *
-     * This method does not sanitize the capabilities ; instead, use
-     * {@link #getDeclaredCapabilitiesSanitized} to retrieve a sanitized
-     * copy of the capabilities as they were passed here.
-     *
+     * <p>
+     * This method does not sanitize the capabilities before storing them ; instead, use
+     * {@link #getDeclaredCapabilitiesSanitized} to retrieve a sanitized copy of the capabilities
+     * as they were passed here.
+     * <p>
      * This method makes a defensive copy to avoid issues where the passed object is later mutated.
      *
      * @param caps the caps sent by the agent

tests/unit/java/com/android/server/connectivityservice/CSLocalAgentTests.kt

@@ -32,11 +32,11 @@ import android.net.RouteInfo
 import android.os.Build
 import com.android.testutils.DevSdkIgnoreRule
 import com.android.testutils.DevSdkIgnoreRunner
-import com.android.testutils.RecorderCallback
 import com.android.testutils.RecorderCallback.CallbackEntry.Available
 import com.android.testutils.RecorderCallback.CallbackEntry.BlockedStatus
 import com.android.testutils.RecorderCallback.CallbackEntry.CapabilitiesChanged
 import com.android.testutils.RecorderCallback.CallbackEntry.LinkPropertiesChanged
+import com.android.testutils.RecorderCallback.CallbackEntry.Lost
 import com.android.testutils.TestableNetworkCallback
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -65,6 +65,8 @@ private fun lp(iface: String) = LinkProperties().apply {
 class CSLocalAgentTests : CSTest() {
     @Test
     fun testBadAgents() {
+        deps.setBuildSdk(VERSION_V)
+
         assertFailsWith<IllegalArgumentException> {
             Agent(nc = NetworkCapabilities.Builder()
                     .addCapability(NET_CAPABILITY_LOCAL_NETWORK)
@@ -77,6 +79,41 @@ class CSLocalAgentTests : CSTest() {
         }
     }
 
+    @Test
+    fun testStructuralConstraintViolation() {
+        deps.setBuildSdk(VERSION_V)
+
+        val cb = TestableNetworkCallback()
+        cm.requestNetwork(NetworkRequest.Builder()
+                .clearCapabilities()
+                .build(),
+                cb)
+        val agent = Agent(nc = NetworkCapabilities.Builder()
+                .addCapability(NET_CAPABILITY_LOCAL_NETWORK)
+                .build(),
+                lnc = LocalNetworkConfig.Builder().build())
+        agent.connect()
+        cb.expect<Available>(agent.network)
+        cb.expect<CapabilitiesChanged>(agent.network)
+        cb.expect<LinkPropertiesChanged>(agent.network)
+        cb.expect<BlockedStatus>(agent.network)
+        agent.sendNetworkCapabilities(NetworkCapabilities.Builder().build())
+        cb.expect<Lost>(agent.network)
+
+        val agent2 = Agent(nc = NetworkCapabilities.Builder()
+                .build(),
+                lnc = null)
+        agent2.connect()
+        cb.expect<Available>(agent2.network)
+        cb.expect<CapabilitiesChanged>(agent2.network)
+        cb.expect<LinkPropertiesChanged>(agent2.network)
+        cb.expect<BlockedStatus>(agent2.network)
+        agent2.sendNetworkCapabilities(NetworkCapabilities.Builder()
+                .addCapability(NET_CAPABILITY_LOCAL_NETWORK)
+                .build())
+        cb.expect<Lost>(agent2.network)
+    }
+
     @Test
     fun testUpdateLocalAgentConfig() {
         deps.setBuildSdk(VERSION_V)

tests/unit/java/com/android/server/connectivityservice/base/CSAgentWrapper.kt

@@ -169,4 +169,5 @@ class CSAgentWrapper(
 
     fun unregisterAfterReplacement(timeoutMs: Int) = agent.unregisterAfterReplacement(timeoutMs)
     fun sendLocalNetworkConfig(lnc: LocalNetworkConfig) = agent.sendLocalNetworkConfig(lnc)
+    fun sendNetworkCapabilities(nc: NetworkCapabilities) = agent.sendNetworkCapabilities(nc)
 }