From 5759bf45a59f31d04d6bca8191e70c1c44e71c5b Mon Sep 17 00:00:00 2001 From: Motomu Utsumi Date: Mon, 7 Apr 2025 21:12:06 +0900 Subject: [PATCH] Update enforceMeteredApnPolicy to follow the compat flag On V+, ConnectivityService uses uid blocking state in BpfMap instead of asking NetworkPolicyManager. But, there was one behavior difference because NetworkPolicyManager considers the network is always not blocked for apps without Internet permission. To ensure backward compatibility, the NETWORK_BLOCKED_WITHOUT_INTERNET_PERMISSION flag was introduced in aosp/3076606, aosp/3088285. This CL updates enforceMeteredApnPolicy to respect this flag for backward compatibility. Test: CSEnforceMeteredApnPolicy Bug: 404428733 Bug: 370383309 Flag: EXEMPT bugfix Change-Id: I631f4f840818e1d40f20fccae1ec1a2cbbb6a166 --- .../ConnectivityCompatChanges.java | 2 + .../android/server/ConnectivityService.java | 19 ++- .../CSEnforceMeteredApnPolicy.kt | 129 ++++++++++++++++++ 3 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 tests/unit/java/com/android/server/connectivityservice/CSEnforceMeteredApnPolicy.kt diff --git a/framework/src/android/net/connectivity/ConnectivityCompatChanges.java b/framework/src/android/net/connectivity/ConnectivityCompatChanges.java index 27d605acf9..c3279306f9 100644 --- a/framework/src/android/net/connectivity/ConnectivityCompatChanges.java +++ b/framework/src/android/net/connectivity/ConnectivityCompatChanges.java @@ -111,6 +111,8 @@ public final class ConnectivityCompatChanges { * {@link android.net.ConnectivityManager#getActiveNetwork()} always returns null. * {@link android.net.ConnectivityManager.NetworkCallback#onBlockedStatusChanged()} is always * called with blocked=true. + * {@link android.net.ConnectivityManager#requestNetwork} can not bring up metered network if + * the uid is restricted from using metered networks. *

* For backwards compatibility, apps running on older releases, or targeting older SDK levels, * network access from apps without {@link android.Manifest.permission#INTERNET} permission is diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 11c6f1fb7f..2f51744e00 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -8849,10 +8849,21 @@ public class ConnectivityService extends IConnectivityManager.Stub { // Policy already enforced. return; } - final boolean isRestrictedOnMeteredNetworks = mDeps.isAtLeastV() - ? mBpfNetMaps.isUidRestrictedOnMeteredNetworks(uid) - : BinderUtils.withCleanCallingIdentity(() -> - mPolicyManager.isUidRestrictedOnMeteredNetworks(uid)); + final boolean isRestrictedOnMeteredNetworks; + if (mDeps.isAtLeastV()) { + if (mDeps.isChangeEnabled(NETWORK_BLOCKED_WITHOUT_INTERNET_PERMISSION, uid)) { + isRestrictedOnMeteredNetworks = + mBpfNetMaps.isUidRestrictedOnMeteredNetworks(uid); + } else { + // If the change is disabled and the uid does not have Internet permission, + // uid is considered to be allowed to bring up metered networks. + isRestrictedOnMeteredNetworks = hasInternetPermission(uid) + && mBpfNetMaps.isUidRestrictedOnMeteredNetworks(uid); + } + } else { + isRestrictedOnMeteredNetworks = BinderUtils.withCleanCallingIdentity(() -> + mPolicyManager.isUidRestrictedOnMeteredNetworks(uid)); + } if (isRestrictedOnMeteredNetworks) { // If UID is restricted, don't allow them to bring up metered APNs. networkCapabilities.addCapability(NET_CAPABILITY_NOT_METERED); diff --git a/tests/unit/java/com/android/server/connectivityservice/CSEnforceMeteredApnPolicy.kt b/tests/unit/java/com/android/server/connectivityservice/CSEnforceMeteredApnPolicy.kt new file mode 100644 index 0000000000..776d5c649a --- /dev/null +++ b/tests/unit/java/com/android/server/connectivityservice/CSEnforceMeteredApnPolicy.kt @@ -0,0 +1,129 @@ +/* + * Copyright (C) 2025 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.INetd.PERMISSION_INTERNET +import android.net.INetd.PERMISSION_NONE +import android.net.NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED +import android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED +import android.net.NetworkCapabilities.TRANSPORT_CELLULAR +import android.net.NetworkProvider +import android.net.NetworkRequest +import android.net.NetworkScore +import android.net.connectivity.ConnectivityCompatChanges.NETWORK_BLOCKED_WITHOUT_INTERNET_PERMISSION +import android.os.Build +import android.os.Process +import androidx.test.filters.SmallTest +import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo +import com.android.testutils.DevSdkIgnoreRunner +import com.android.testutils.TestableNetworkCallback +import com.android.testutils.TestableNetworkOfferCallback +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized +import org.mockito.Mockito.doReturn + +private const val TIMEOUT_MS = 5_000L +private const val NO_CB_TIMEOUT_MS = 200L + +private fun cellNc() = nc( + TRANSPORT_CELLULAR, + NET_CAPABILITY_NOT_SUSPENDED, + NET_CAPABILITY_NOT_VCN_MANAGED +) + +private fun cellRequest() = NetworkRequest.Builder() + .addTransportType(TRANSPORT_CELLULAR) + .build() + +@RunWith(DevSdkIgnoreRunner::class) +@SmallTest +@IgnoreUpTo(Build.VERSION_CODES.R) +class CSEnforceMeteredApnPolicy : CSTest() { + @Parameterized.Parameter(0) lateinit var params: TestParams + + data class TestParams( + val changeEnabled: Boolean, + val hasInternetPermission: Boolean, + val expectRequestMeteredNetwork: Boolean + ) + + companion object { + @JvmStatic + @Parameterized.Parameters + fun arguments() = listOf( + // For backwards compatibility, if the change is disabled and the uid does not have + // the internet permission, the uid can request metered network even if the uid is + // restricted from using metered networks. + TestParams( + changeEnabled = false, + hasInternetPermission = false, + expectRequestMeteredNetwork = true + ), + // If the uid has the internet permission and the uid is restricted from using + // metered network, the uid cannot request metered network even if the change is + // disabled. + TestParams( + changeEnabled = false, + hasInternetPermission = true, + expectRequestMeteredNetwork = false + ), + // If the change is enabled and the uid is restricted from using metered network, + // the uid cannot request metered network regardless of the internet permission. + TestParams( + changeEnabled = true, + hasInternetPermission = false, + expectRequestMeteredNetwork = false + ), + TestParams( + changeEnabled = true, + hasInternetPermission = true, + expectRequestMeteredNetwork = false + ) + ) + } + + @Test + fun testEnforceMeteredApnPolicy() { + deps.setBuildSdk(VERSION_V) + deps.setChangeIdEnabled(params.changeEnabled, NETWORK_BLOCKED_WITHOUT_INTERNET_PERMISSION) + doReturn(true).`when`(bpfNetMaps).isUidRestrictedOnMeteredNetworks(Process.myUid()) + doReturn(if (params.hasInternetPermission) PERMISSION_INTERNET else PERMISSION_NONE ) + .`when`(bpfNetMaps).getNetPermForUid(Process.myUid()) + + val provider = NetworkProvider(context, csHandlerThread.looper, "Cell provider") + cm.registerNetworkProvider(provider) + val offerCb = TestableNetworkOfferCallback(TIMEOUT_MS, NO_CB_TIMEOUT_MS) + provider.registerNetworkOffer( + NetworkScore.Builder().build(), + cellNc(), + Runnable::run, + offerCb + ) + val cb = TestableNetworkCallback() + cm.requestNetwork(cellRequest(), cb) + + if (params.expectRequestMeteredNetwork) { + offerCb.expectOnNetworkNeeded(cellNc()) + } else { + offerCb.assertNoCallback() + } + + cm.unregisterNetworkCallback(cb) + provider.unregisterNetworkOffer(offerCb) + } +}