Merge changes Ibb8d33b7,Ie168fe1f,I9f699b63 into main

* changes:
  Prepare exposing Network{Request,Caps}.forbiddenCapabilities
  Add a keep connected for test reason
  Improvements to CSTest : legacy type, wait for LOST, permissions

common/TrunkStable.bp

@@ -19,3 +19,8 @@ aconfig_declarations {
     package: "com.android.net.flags",
     srcs: ["flags.aconfig"],
 }
+
+java_aconfig_library {
+    name: "connectivity_flags_aconfig_lib",
+    aconfig_declarations: "com.android.net.flags-aconfig",
+}

common/flags.aconfig

@@ -6,3 +6,10 @@ flag {
   description: "NetworkActivityTracker tracks multiple networks including non default networks"
   bug: "267870186"
 }
+
+flag {
+  name: "forbidden_capability"
+  namespace: "android_core_networking"
+  description: "This flag controls the forbidden capability API"
+  bug: "302997505"
+}

framework/src/android/net/NetworkCapabilities.java

@@ -29,6 +29,9 @@ import android.annotation.SuppressLint;
 import android.annotation.SystemApi;
 import android.compat.annotation.UnsupportedAppUsage;
 import android.net.ConnectivityManager.NetworkCallback;
+// Can't be imported because aconfig tooling doesn't exist on udc-mainline-prod yet
+// See inner class Flags which mimics this for the time being
+// import android.net.flags.Flags;
 import android.os.Build;
 import android.os.Parcel;
 import android.os.Parcelable;
@@ -121,6 +124,14 @@ import java.util.StringJoiner;
 public final class NetworkCapabilities implements Parcelable {
     private static final String TAG = "NetworkCapabilities";
 
+    // TODO : remove this class when udc-mainline-prod is abandoned and android.net.flags.Flags is
+    // available here
+    /** @hide */
+    public static class Flags {
+        static final String FLAG_FORBIDDEN_CAPABILITY =
+                "com.android.net.flags.forbidden_capability";
+    }
+
     /**
      * Mechanism to support redaction of fields in NetworkCapabilities that are guarded by specific
      * app permissions.
@@ -793,6 +804,10 @@ public final class NetworkCapabilities implements Parcelable {
      * Adds the given capability to this {@code NetworkCapability} instance.
      * Note that when searching for a network to satisfy a request, all capabilities
      * requested must be satisfied.
+     * <p>
+     * If the capability was previously added to the list of forbidden capabilities (either
+     * by default or added using {@link #addForbiddenCapability(int)}), then it will be removed
+     * from the list of forbidden capabilities as well.
      *
      * @param capability the capability to be added.
      * @return This NetworkCapabilities instance, to facilitate chaining.
@@ -801,8 +816,7 @@ public final class NetworkCapabilities implements Parcelable {
     public @NonNull NetworkCapabilities addCapability(@NetCapability int capability) {
         // If the given capability was previously added to the list of forbidden capabilities
         // then the capability will also be removed from the list of forbidden capabilities.
-        // TODO: Consider adding forbidden capabilities to the public API and mention this
+        // TODO: Add forbidden capabilities to the public API
-        // in the documentation.
         checkValidCapability(capability);
         mNetworkCapabilities |= 1L << capability;
         // remove from forbidden capability list
@@ -901,6 +915,8 @@ public final class NetworkCapabilities implements Parcelable {
      * @return an array of forbidden capability values for this instance.
      * @hide
      */
+    @NonNull
+    // TODO : @FlaggedApi(Flags.FLAG_FORBIDDEN_CAPABILITY) and public
     public @NetCapability int[] getForbiddenCapabilities() {
         return BitUtils.unpackBits(mForbiddenNetworkCapabilities);
     }
@@ -1000,7 +1016,7 @@ public final class NetworkCapabilities implements Parcelable {
     /**
      * Tests for the presence of a capability on this instance.
      *
-     * @param capability the capabilities to be tested for.
+     * @param capability the capability to be tested for.
      * @return {@code true} if set on this instance.
      */
     public boolean hasCapability(@NetCapability int capability) {
@@ -1008,8 +1024,15 @@ public final class NetworkCapabilities implements Parcelable {
                 && ((mNetworkCapabilities & (1L << capability)) != 0);
     }
 
-    /** @hide */
+    /**
+     * Tests for the presence of a forbidden capability on this instance.
+     *
+     * @param capability the capability to be tested for.
+     * @return {@code true} if this capability is set forbidden on this instance.
+     * @hide
+     */
     @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES)
+    // TODO : @FlaggedApi(Flags.FLAG_FORBIDDEN_CAPABILITY) and public
     public boolean hasForbiddenCapability(@NetCapability int capability) {
         return isValidCapability(capability)
                 && ((mForbiddenNetworkCapabilities & (1L << capability)) != 0);
@@ -2888,6 +2911,44 @@ public final class NetworkCapabilities implements Parcelable {
             return this;
         }
 
+        /**
+         * Adds the given capability to the list of forbidden capabilities.
+         *
+         * A network with a capability will not match a {@link NetworkCapabilities} or
+         * {@link NetworkRequest} which has said capability set as forbidden. For example, if
+         * a request has NET_CAPABILITY_INTERNET in the list of forbidden capabilities, networks
+         * with NET_CAPABILITY_INTERNET will not match the request.
+         *
+         * If the capability was previously added to the list of required capabilities (for
+         * example, it was there by default or added using {@link #addCapability(int)} method), then
+         * it will be removed from the list of required capabilities as well.
+         *
+         * @param capability the capability
+         * @return this builder
+         * @hide
+         */
+        @NonNull
+        // TODO : @FlaggedApi(Flags.FLAG_FORBIDDEN_CAPABILITY) and public
+        public Builder addForbiddenCapability(@NetCapability final int capability) {
+            mCaps.addForbiddenCapability(capability);
+            return this;
+        }
+
+        /**
+         * Removes the given capability from the list of forbidden capabilities.
+         *
+         * @see #addForbiddenCapability(int)
+         * @param capability the capability
+         * @return this builder
+         * @hide
+         */
+        @NonNull
+        // TODO : @FlaggedApi(Flags.FLAG_FORBIDDEN_CAPABILITY) and public
+        public Builder removeForbiddenCapability(@NetCapability final int capability) {
+            mCaps.removeForbiddenCapability(capability);
+            return this;
+        }
+
         /**
          * Adds the given enterprise capability identifier.
          * Note that when searching for a network to satisfy a request, all capabilities identifier
@@ -3235,4 +3296,4 @@ public final class NetworkCapabilities implements Parcelable {
             return new NetworkCapabilities(mCaps);
         }
     }
-}
+}

framework/src/android/net/NetworkRequest.java

@@ -39,6 +39,8 @@ import android.annotation.RequiresPermission;
 import android.annotation.SuppressLint;
 import android.annotation.SystemApi;
 import android.compat.annotation.UnsupportedAppUsage;
+// TODO : replace with android.net.flags.Flags when aconfig is supported on udc-mainline-prod
+// import android.net.NetworkCapabilities.Flags;
 import android.net.NetworkCapabilities.NetCapability;
 import android.net.NetworkCapabilities.Transport;
 import android.os.Build;
@@ -408,6 +410,7 @@ public class NetworkRequest implements Parcelable {
         @NonNull
         @SuppressLint("MissingGetterMatchingBuilder")
         @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES)
+        // TODO : @FlaggedApi(Flags.FLAG_FORBIDDEN_CAPABILITY) and public
         public Builder addForbiddenCapability(@NetworkCapabilities.NetCapability int capability) {
             mNetworkCapabilities.addForbiddenCapability(capability);
             return this;
@@ -424,6 +427,7 @@ public class NetworkRequest implements Parcelable {
         @NonNull
         @SuppressLint("BuilderSetStyle")
         @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES)
+        // TODO : @FlaggedApi(Flags.FLAG_FORBIDDEN_CAPABILITY) and public
         public Builder removeForbiddenCapability(
                 @NetworkCapabilities.NetCapability int capability) {
             mNetworkCapabilities.removeForbiddenCapability(capability);
@@ -433,6 +437,7 @@ public class NetworkRequest implements Parcelable {
         /**
          * Completely clears all the {@code NetworkCapabilities} from this builder instance,
          * removing even the capabilities that are set by default when the object is constructed.
+         * Also removes any set forbidden capabilities.
          *
          * @return The builder to facilitate chaining.
          */
@@ -721,6 +726,7 @@ public class NetworkRequest implements Parcelable {
      * @hide
      */
     @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES)
+    // TODO : @FlaggedApi(Flags.FLAG_FORBIDDEN_CAPABILITY) and public instead of @SystemApi
     public boolean hasForbiddenCapability(@NetCapability int capability) {
         return networkCapabilities.hasForbiddenCapability(capability);
     }
@@ -843,6 +849,7 @@ public class NetworkRequest implements Parcelable {
      */
     @NonNull
     @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES)
+    // TODO : @FlaggedApi(Flags.FLAG_FORBIDDEN_CAPABILITY) and public instead of @SystemApi
     public @NetCapability int[] getForbiddenCapabilities() {
         // No need to make a defensive copy here as NC#getForbiddenCapabilities() already returns
         // a new array.

framework/src/android/net/NetworkScore.java

@@ -44,7 +44,8 @@ public final class NetworkScore implements Parcelable {
     @Retention(RetentionPolicy.SOURCE)
     @IntDef(value = {
             KEEP_CONNECTED_NONE,
-            KEEP_CONNECTED_FOR_HANDOVER
+            KEEP_CONNECTED_FOR_HANDOVER,
+            KEEP_CONNECTED_FOR_TEST
     })
     public @interface KeepConnectedReason { }
 
@@ -57,6 +58,12 @@ public final class NetworkScore implements Parcelable {
      * is being considered for handover.
      */
     public static final int KEEP_CONNECTED_FOR_HANDOVER = 1;
+    /**
+     * Keep this network connected even if there is no outstanding request for it, because it
+     * is used in a test and it's not necessarily easy to file the right request for it.
+     * @hide
+     */
+    public static final int KEEP_CONNECTED_FOR_TEST = 2;
 
     // Agent-managed policies
     // This network should lose to a wifi that has ever been validated

tests/cts/net/src/android/net/cts/NetworkRequestTest.java

@@ -104,6 +104,23 @@ public class NetworkRequestTest {
         verifyNoCapabilities(nr);
     }
 
+    @Test @IgnoreUpTo(Build.VERSION_CODES.R)
+    public void testForbiddenCapabilities() {
+        final NetworkRequest.Builder builder = new NetworkRequest.Builder();
+        builder.addForbiddenCapability(NET_CAPABILITY_MMS);
+        assertTrue(builder.build().hasForbiddenCapability(NET_CAPABILITY_MMS));
+        builder.removeForbiddenCapability(NET_CAPABILITY_MMS);
+        assertFalse(builder.build().hasCapability(NET_CAPABILITY_MMS));
+        builder.addCapability(NET_CAPABILITY_MMS);
+        assertFalse(builder.build().hasForbiddenCapability(NET_CAPABILITY_MMS));
+        assertTrue(builder.build().hasCapability(NET_CAPABILITY_MMS));
+        builder.addForbiddenCapability(NET_CAPABILITY_MMS);
+        assertTrue(builder.build().hasForbiddenCapability(NET_CAPABILITY_MMS));
+        assertFalse(builder.build().hasCapability(NET_CAPABILITY_MMS));
+        builder.clearCapabilities();
+        verifyNoCapabilities(builder.build());
+    }
+
     @Test @IgnoreUpTo(Build.VERSION_CODES.Q)
     public void testTemporarilyNotMeteredCapability() {
         assertTrue(new NetworkRequest.Builder()

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

@@ -0,0 +1,58 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server
+
+import android.net.NetworkRequest
+import android.net.NetworkScore
+import android.net.NetworkScore.KEEP_CONNECTED_FOR_TEST
+import android.os.Build
+import androidx.test.filters.SmallTest
+import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo
+import com.android.testutils.DevSdkIgnoreRunner
+import com.android.testutils.RecorderCallback.CallbackEntry.Lost
+import com.android.testutils.TestableNetworkCallback
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(DevSdkIgnoreRunner::class)
+@SmallTest
+@IgnoreUpTo(Build.VERSION_CODES.TIRAMISU)
+class CSKeepConnectedTest : CSTest() {
+    @Test
+    fun testKeepConnectedForTest() {
+        val keepAgent = Agent(score = FromS(NetworkScore.Builder()
+                .setKeepConnectedReason(KEEP_CONNECTED_FOR_TEST)
+                .build()))
+        val dontKeepAgent = Agent()
+        doTestKeepConnected(keepAgent, dontKeepAgent)
+    }
+
+    fun doTestKeepConnected(keepAgent: CSAgentWrapper, dontKeepAgent: CSAgentWrapper) {
+        val cb = TestableNetworkCallback()
+        cm.registerNetworkCallback(NetworkRequest.Builder().clearCapabilities().build(), cb)
+
+        keepAgent.connect()
+        dontKeepAgent.connect()
+
+        cb.expectAvailableCallbacks(keepAgent.network, validated = false)
+        cb.expectAvailableCallbacks(dontKeepAgent.network, validated = false)
+
+        // After the nascent timer, the agent without keep connected gets lost.
+        cb.expect<Lost>(dontKeepAgent.network)
+        cb.assertNoCallback()
+    }
+}

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

@@ -35,6 +35,7 @@ import android.net.networkstack.NetworkStackClientBase
 import android.os.HandlerThread
 import com.android.modules.utils.build.SdkLevel
 import com.android.testutils.RecorderCallback.CallbackEntry.Available
+import com.android.testutils.RecorderCallback.CallbackEntry.Lost
 import com.android.testutils.TestableNetworkCallback
 import org.mockito.ArgumentCaptor
 import org.mockito.ArgumentMatchers.any
@@ -47,6 +48,8 @@ import java.util.concurrent.atomic.AtomicInteger
 import kotlin.test.assertEquals
 import kotlin.test.fail
 
+const val SHORT_TIMEOUT_MS = 200L
+
 private inline fun <reified T> ArgumentCaptor() = ArgumentCaptor.forClass(T::class.java)
 
 private val agentCounter = AtomicInteger(1)
@@ -60,6 +63,7 @@ private fun nextAgentId() = agentCounter.getAndIncrement()
  */
 class CSAgentWrapper(
         val context: Context,
+        val deps: ConnectivityService.Dependencies,
         csHandlerThread: HandlerThread,
         networkStack: NetworkStackClientBase,
         nac: NetworkAgentConfig,
@@ -125,9 +129,10 @@ class CSAgentWrapper(
 
     fun connect() {
         val mgr = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
-        val request = NetworkRequest.Builder().clearCapabilities()
+        val request = NetworkRequest.Builder().apply {
-                .addTransportType(nc.transportTypes[0])
+            clearCapabilities()
-                .build()
+            if (nc.transportTypes.isNotEmpty()) addTransportType(nc.transportTypes[0])
+        }.build()
         val cb = TestableNetworkCallback()
         mgr.registerNetworkCallback(request, cb)
         agent.markConnected()
@@ -149,6 +154,15 @@ class CSAgentWrapper(
     }
 
     fun disconnect() {
+        val mgr = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
+        val request = NetworkRequest.Builder().apply {
+            clearCapabilities()
+            if (nc.transportTypes.isNotEmpty()) addTransportType(nc.transportTypes[0])
+        }.build()
+        val cb = TestableNetworkCallback(timeoutMs = SHORT_TIMEOUT_MS)
+        mgr.registerNetworkCallback(request, cb)
+        cb.eventuallyExpect<Available> { it.network == agent.network }
         agent.unregister()
+        cb.eventuallyExpect<Lost> { it.network == agent.network }
     }
 }

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

@@ -30,6 +30,12 @@ import android.net.LinkProperties
 import android.net.NetworkAgentConfig
 import android.net.NetworkCapabilities
 import android.net.NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED
+import android.net.NetworkCapabilities.TRANSPORT_BLUETOOTH
+import android.net.NetworkCapabilities.TRANSPORT_CELLULAR
+import android.net.NetworkCapabilities.TRANSPORT_ETHERNET
+import android.net.NetworkCapabilities.TRANSPORT_TEST
+import android.net.NetworkCapabilities.TRANSPORT_VPN
+import android.net.NetworkCapabilities.TRANSPORT_WIFI
 import android.net.NetworkPolicyManager
 import android.net.NetworkProvider
 import android.net.NetworkScore
@@ -79,6 +85,17 @@ internal const val VERSION_U = 4
 internal const val VERSION_V = 5
 internal const val VERSION_MAX = VERSION_V
 
+private fun NetworkCapabilities.getLegacyType() =
+        when (transportTypes.getOrElse(0) { TRANSPORT_WIFI }) {
+            TRANSPORT_BLUETOOTH -> ConnectivityManager.TYPE_BLUETOOTH
+            TRANSPORT_CELLULAR -> ConnectivityManager.TYPE_MOBILE
+            TRANSPORT_ETHERNET -> ConnectivityManager.TYPE_ETHERNET
+            TRANSPORT_TEST -> ConnectivityManager.TYPE_TEST
+            TRANSPORT_VPN -> ConnectivityManager.TYPE_VPN
+            TRANSPORT_WIFI -> ConnectivityManager.TYPE_WIFI
+            else -> ConnectivityManager.TYPE_NONE
+        }
+
 /**
  * Base class for tests testing ConnectivityService and its satellites.
  *
@@ -127,7 +144,7 @@ open class CSTest {
     val networkStack = mock<NetworkStackClientBase>()
     val csHandlerThread = HandlerThread("CSTestHandler")
     val sysResources = mock<Resources>().also { initMockedResources(it) }
-    val packageManager = makeMockPackageManager()
+    val packageManager = makeMockPackageManager(instrumentationContext)
     val connResources = makeMockConnResources(sysResources, packageManager)
 
     val netd = mock<INetd>()
@@ -272,12 +289,12 @@ open class CSTest {
     // Network agents. See CSAgentWrapper. This class contains utility methods to simplify
     // creation.
     fun Agent(
-            nac: NetworkAgentConfig = emptyAgentConfig(),
             nc: NetworkCapabilities = defaultNc(),
+            nac: NetworkAgentConfig = emptyAgentConfig(nc.getLegacyType()),
             lp: LinkProperties = defaultLp(),
             score: FromS<NetworkScore> = defaultScore(),
             provider: NetworkProvider? = null
-    ) = CSAgentWrapper(context, csHandlerThread, networkStack, nac, nc, lp, score, provider)
+    ) = CSAgentWrapper(context, deps, csHandlerThread, networkStack, nac, nc, lp, score, provider)
 
     fun Agent(vararg transports: Int, lp: LinkProperties = defaultLp()): CSAgentWrapper {
         val nc = NetworkCapabilities.Builder().apply {

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

@@ -38,6 +38,7 @@ import android.net.NetworkCapabilities
 import android.net.NetworkScore
 import android.net.RouteInfo
 import android.net.metrics.IpConnectivityLog
+import android.os.Binder
 import android.os.Handler
 import android.os.HandlerThread
 import android.os.SystemClock
@@ -54,20 +55,23 @@ import com.android.server.ConnectivityService.Dependencies
 import com.android.server.connectivity.ConnectivityResources
 import org.mockito.ArgumentMatchers
 import org.mockito.ArgumentMatchers.any
+import org.mockito.ArgumentMatchers.anyInt
 import org.mockito.ArgumentMatchers.anyLong
 import org.mockito.ArgumentMatchers.anyString
 import org.mockito.ArgumentMatchers.argThat
 import org.mockito.ArgumentMatchers.eq
 import org.mockito.Mockito
 import org.mockito.Mockito.doAnswer
-import org.mockito.Mockito.doReturn
 import org.mockito.Mockito.doNothing
+import org.mockito.Mockito.doReturn
 import kotlin.test.fail
 
 internal inline fun <reified T> mock() = Mockito.mock(T::class.java)
 internal inline fun <reified T> any() = any(T::class.java)
 
-internal fun emptyAgentConfig() = NetworkAgentConfig.Builder().build()
+internal fun emptyAgentConfig(legacyType: Int) = NetworkAgentConfig.Builder()
+        .setLegacyType(legacyType)
+        .build()
 
 internal fun defaultNc() = NetworkCapabilities.Builder()
         // Add sensible defaults for agents that don't want to care
@@ -98,9 +102,22 @@ internal fun makeActivityManager() = mock<ActivityManager>().also {
     }
 }
 
-internal fun makeMockPackageManager() = mock<PackageManager>().also { pm ->
+internal fun makeMockPackageManager(realContext: Context) = mock<PackageManager>().also { pm ->
     val supported = listOf(FEATURE_WIFI, FEATURE_WIFI_DIRECT, FEATURE_BLUETOOTH, FEATURE_ETHERNET)
     doReturn(true).`when`(pm).hasSystemFeature(argThat { supported.contains(it) })
+    val myPackageName = realContext.packageName
+    val myPackageInfo = realContext.packageManager.getPackageInfo(myPackageName,
+            PackageManager.GET_PERMISSIONS)
+    // Very high version code so that the checks for the module version will always
+    // say that it is recent enough. This is the most sensible default, but if some
+    // test needs to test with different version codes they can re-mock this with a
+    // different value.
+    myPackageInfo.longVersionCode = 9999999L
+    doReturn(arrayOf(myPackageName)).`when`(pm).getPackagesForUid(Binder.getCallingUid())
+    doReturn(myPackageInfo).`when`(pm).getPackageInfoAsUser(
+            eq(myPackageName), anyInt(), eq(UserHandle.getCallingUserId()))
+    doReturn(listOf(myPackageInfo)).`when`(pm)
+            .getInstalledPackagesAsUser(eq(PackageManager.GET_PERMISSIONS), anyInt())
 }
 
 internal fun makeMockConnResources(resources: Resources, pm: PackageManager) = mock<Context>().let {