From cc409153b48762ecf77d7fe182d0a5a7243a4794 Mon Sep 17 00:00:00 2001 From: Anton Kulakov Date: Fri, 13 Oct 2023 15:04:01 +0000 Subject: [PATCH] Revert "Tell netd about local networks" Revert submission 2777667 Reason for revert: DroidMonitor-triggered revert due to breakage, bug b/305187495 Reverted changes: /q/submissionid:2777667 Bug: 305187495 Change-Id: Icb46558d17b56f9f6fd402f1489de9ed2b8f7644 --- .../android/server/ConnectivityService.java | 27 +---- .../server/connectivity/NetworkAgentInfo.java | 14 +-- .../CSLocalAgentCreationTests.kt | 114 ------------------ 3 files changed, 7 insertions(+), 148 deletions(-) delete mode 100644 tests/unit/java/com/android/server/connectivityservice/CSLocalAgentCreationTests.kt diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 6aee6f8b9c..8475110440 100755 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -19,7 +19,6 @@ package com.android.server; import static android.Manifest.permission.RECEIVE_DATA_ACTIVITY_CHANGE; import static android.app.ActivityManager.UidFrozenStateChangedCallback.UID_FROZEN_STATE_FROZEN; import static android.content.pm.PackageManager.FEATURE_BLUETOOTH; -import static android.content.pm.PackageManager.FEATURE_LEANBACK; import static android.content.pm.PackageManager.FEATURE_WATCH; import static android.content.pm.PackageManager.FEATURE_WIFI; import static android.content.pm.PackageManager.FEATURE_WIFI_DIRECT; @@ -68,7 +67,6 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL; import static android.net.NetworkCapabilities.NET_CAPABILITY_ENTERPRISE; import static android.net.NetworkCapabilities.NET_CAPABILITY_FOREGROUND; import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; -import static android.net.NetworkCapabilities.NET_CAPABILITY_LOCAL_NETWORK; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_CONGESTED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED; @@ -5008,10 +5006,7 @@ public class ConnectivityService extends IConnectivityManager.Stub !nai.networkAgentConfig.allowBypass /* secure */, getVpnType(nai), nai.networkAgentConfig.excludeLocalRouteVpn); } else { - final boolean hasLocalCap = - nai.networkCapabilities.hasCapability(NET_CAPABILITY_LOCAL_NETWORK); - config = new NativeNetworkConfig(nai.network.getNetId(), - hasLocalCap ? NativeNetworkType.PHYSICAL_LOCAL : NativeNetworkType.PHYSICAL, + config = new NativeNetworkConfig(nai.network.getNetId(), NativeNetworkType.PHYSICAL, getNetworkPermission(nai.networkCapabilities), false /* secure */, VpnManager.TYPE_VPN_NONE, @@ -8061,18 +8056,6 @@ public class ConnectivityService extends IConnectivityManager.Stub return nai == getDefaultNetwork(); } - /** - * Returns whether local agents are supported on this device. - * - * Local agents are supported from U on TVs, and from V on all devices. - */ - @VisibleForTesting - public boolean areLocalAgentsSupported() { - final PackageManager pm = mContext.getPackageManager(); - // Local agents are supported starting on U on TVs and on V on everything else. - return mDeps.isAtLeastV() || (mDeps.isAtLeastU() && pm.hasSystemFeature(FEATURE_LEANBACK)); - } - /** * Register a new agent with ConnectivityService to handle a network. * @@ -8102,12 +8085,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } else { enforceNetworkFactoryPermission(); } - final boolean hasLocalCap = - networkCapabilities.hasCapability(NET_CAPABILITY_LOCAL_NETWORK); - if (hasLocalCap && !areLocalAgentsSupported()) { - // Before U, netd doesn't support PHYSICAL_LOCAL networks so this can't work. - throw new IllegalArgumentException("Local agents are not supported in this version"); - } final int uid = mDeps.getCallingUid(); final long token = Binder.clearCallingIdentity(); @@ -9213,7 +9190,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // are Type.LISTEN, but should not have NetworkCallbacks invoked. return; } - final Bundle bundle = new Bundle(); + Bundle bundle = new Bundle(); // TODO b/177608132: make sure callbacks are indexed by NRIs and not NetworkRequest objects. // TODO: check if defensive copies of data is needed. final NetworkRequest nrForCallback = nri.getNetworkRequestForCallback(); diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java index 8d0d7116c9..0f72cd462d 100644 --- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java +++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java @@ -64,6 +64,7 @@ import android.util.SparseArray; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.WakeupMessage; +import com.android.modules.utils.build.SdkLevel; import com.android.server.ConnectivityService; import java.io.PrintWriter; @@ -469,8 +470,8 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable { + networkCapabilities.getOwnerUid() + " to " + nc.getOwnerUid()); nc.setOwnerUid(networkCapabilities.getOwnerUid()); } - restrictCapabilitiesFromNetworkAgent(nc, creatorUid, mHasAutomotiveFeature, - mConnServiceDeps, carrierPrivilegeAuthenticator); + restrictCapabilitiesFromNetworkAgent( + nc, creatorUid, mHasAutomotiveFeature, carrierPrivilegeAuthenticator); return nc; } @@ -600,7 +601,6 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable { private static final String TAG = ConnectivityService.class.getSimpleName(); private static final boolean VDBG = false; private final ConnectivityService mConnService; - private final ConnectivityService.Dependencies mConnServiceDeps; private final Context mContext; private final Handler mHandler; private final QosCallbackTracker mQosCallbackTracker; @@ -628,7 +628,6 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable { networkCapabilities = nc; networkAgentConfig = config; mConnService = connService; - mConnServiceDeps = deps; setScore(score); // uses members connService, networkCapabilities and networkAgentConfig clatd = new Nat464Xlat(this, netd, dnsResolver, deps); mContext = context; @@ -1519,26 +1518,23 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable { */ public static void restrictCapabilitiesFromNetworkAgent(@NonNull final NetworkCapabilities nc, final int creatorUid, final boolean hasAutomotiveFeature, - @NonNull final ConnectivityService.Dependencies deps, @Nullable final CarrierPrivilegeAuthenticator authenticator) { if (nc.hasTransport(TRANSPORT_TEST)) { nc.restrictCapabilitiesForTestNetwork(creatorUid); } - if (!areAllowedUidsAcceptableFromNetworkAgent( - nc, hasAutomotiveFeature, deps, authenticator)) { + if (!areAllowedUidsAcceptableFromNetworkAgent(nc, hasAutomotiveFeature, authenticator)) { nc.setAllowedUids(new ArraySet<>()); } } private static boolean areAllowedUidsAcceptableFromNetworkAgent( @NonNull final NetworkCapabilities nc, final boolean hasAutomotiveFeature, - @NonNull final ConnectivityService.Dependencies deps, @Nullable final CarrierPrivilegeAuthenticator carrierPrivilegeAuthenticator) { // NCs without access UIDs are fine. if (!nc.hasAllowedUids()) return true; // S and below must never accept access UIDs, even if an agent sends them, because netd // didn't support the required feature in S. - if (!deps.isAtLeastT()) return false; + if (!SdkLevel.isAtLeastT()) return false; // On a non-restricted network, access UIDs make no sense if (nc.hasCapability(NET_CAPABILITY_NOT_RESTRICTED)) return false; diff --git a/tests/unit/java/com/android/server/connectivityservice/CSLocalAgentCreationTests.kt b/tests/unit/java/com/android/server/connectivityservice/CSLocalAgentCreationTests.kt deleted file mode 100644 index 7914e04958..0000000000 --- a/tests/unit/java/com/android/server/connectivityservice/CSLocalAgentCreationTests.kt +++ /dev/null @@ -1,114 +0,0 @@ -/* - * 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.content.pm.PackageManager.FEATURE_LEANBACK -import android.net.INetd -import android.net.NativeNetworkConfig -import android.net.NativeNetworkType -import android.net.NetworkCapabilities -import android.net.NetworkCapabilities.NET_CAPABILITY_LOCAL_NETWORK -import android.net.NetworkRequest -import android.net.NetworkScore -import android.net.NetworkScore.KEEP_CONNECTED_FOR_TEST -import android.net.VpnManager -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.Available -import com.android.testutils.TestableNetworkCallback -import org.junit.Assert.assertEquals -import org.junit.Test -import org.junit.runner.RunWith -import org.junit.runners.Parameterized -import org.mockito.Mockito.doReturn -import org.mockito.Mockito.inOrder -import org.mockito.Mockito.never -import org.mockito.Mockito.timeout -import kotlin.test.assertFailsWith - -private const val TIMEOUT_MS = 2_000L -private const val NO_CALLBACK_TIMEOUT_MS = 200L - -private fun keepConnectedScore() = - FromS(NetworkScore.Builder().setKeepConnectedReason(KEEP_CONNECTED_FOR_TEST).build()) - -@RunWith(DevSdkIgnoreRunner::class) -@SmallTest -@IgnoreUpTo(Build.VERSION_CODES.R) -class CSLocalAgentCreationTests( - private val sdkLevel: Int, - private val isTv: Boolean, - private val addLocalNetCapToRequest: Boolean -) : CSTest() { - companion object { - @JvmStatic - @Parameterized.Parameters - fun arguments() = listOf( - arrayOf(VERSION_V, false /* isTv */, true /* addLocalNetCapToRequest */), - arrayOf(VERSION_V, false /* isTv */, false /* addLocalNetCapToRequest */), - arrayOf(VERSION_V, true /* isTv */, true /* addLocalNetCapToRequest */), - arrayOf(VERSION_V, true /* isTv */, false /* addLocalNetCapToRequest */), - arrayOf(VERSION_U, false /* isTv */, true /* addLocalNetCapToRequest */), - arrayOf(VERSION_U, false /* isTv */, false /* addLocalNetCapToRequest */), - arrayOf(VERSION_U, true /* isTv */, true /* addLocalNetCapToRequest */), - arrayOf(VERSION_U, true /* isTv */, false /* addLocalNetCapToRequest */), - arrayOf(VERSION_T, false /* isTv */, false /* addLocalNetCapToRequest */), - arrayOf(VERSION_T, true /* isTv */, false /* addLocalNetCapToRequest */), - ) - } - - private fun makeNativeNetworkConfigLocal(netId: Int, permission: Int) = - NativeNetworkConfig(netId, NativeNetworkType.PHYSICAL_LOCAL, permission, - false /* secure */, VpnManager.TYPE_VPN_NONE, false /* excludeLocalRoutes */) - - @Test - fun testLocalAgents() { - val netdInOrder = inOrder(netd) - deps.setBuildSdk(sdkLevel) - doReturn(isTv).`when`(packageManager).hasSystemFeature(FEATURE_LEANBACK) - val allNetworksCb = TestableNetworkCallback() - val request = NetworkRequest.Builder() - if (addLocalNetCapToRequest) { - request.addCapability(NET_CAPABILITY_LOCAL_NETWORK) - } - cm.registerNetworkCallback(request.build(), allNetworksCb) - val ncTemplate = NetworkCapabilities.Builder().run { - addTransportType(NetworkCapabilities.TRANSPORT_WIFI) - addCapability(NET_CAPABILITY_LOCAL_NETWORK) - }.build() - val localAgent = if (sdkLevel >= VERSION_V || sdkLevel == VERSION_U && isTv) { - Agent(nc = ncTemplate, score = keepConnectedScore()) - } else { - assertFailsWith { Agent(nc = ncTemplate) } - netdInOrder.verify(netd, never()).networkCreate(any()) - return - } - localAgent.connect() - netdInOrder.verify(netd).networkCreate( - makeNativeNetworkConfigLocal(localAgent.network.netId, INetd.PERMISSION_NONE)) - if (addLocalNetCapToRequest) { - assertEquals(localAgent.network, allNetworksCb.expect().network) - } else { - allNetworksCb.assertNoCallback(NO_CALLBACK_TIMEOUT_MS) - } - cm.unregisterNetworkCallback(allNetworksCb) - localAgent.disconnect() - netdInOrder.verify(netd, timeout(TIMEOUT_MS)).networkDestroy(localAgent.network.netId) - } -}