Revert^2 "Add blocked reason for internet permission"
This reverts commit cb74c08979530b898a7a30137d25b3253d600a69.
Reason for revert: This change is not a cause of b/342557183
Change-Id: Iee8d9b05bbe130a46b627b12da525177b4ee89b2
diff --git a/common/flags.aconfig b/common/flags.aconfig
index 55a96ac..6c3e89d 100644
--- a/common/flags.aconfig
+++ b/common/flags.aconfig
@@ -99,3 +99,11 @@
description: "Flag for oem deny chains blocked reasons API"
bug: "328732146"
}
+
+flag {
+ name: "blocked_reason_network_restricted"
+ is_exported: true
+ namespace: "android_core_networking"
+ description: "Flag for BLOCKED_REASON_NETWORK_RESTRICTED API"
+ bug: "339559837"
+}
diff --git a/framework/api/module-lib-current.txt b/framework/api/module-lib-current.txt
index d233f3e..cd7307f 100644
--- a/framework/api/module-lib-current.txt
+++ b/framework/api/module-lib-current.txt
@@ -51,6 +51,7 @@
field public static final int BLOCKED_REASON_DOZE = 2; // 0x2
field public static final int BLOCKED_REASON_LOCKDOWN_VPN = 16; // 0x10
field public static final int BLOCKED_REASON_LOW_POWER_STANDBY = 32; // 0x20
+ field @FlaggedApi("com.android.net.flags.blocked_reason_network_restricted") public static final int BLOCKED_REASON_NETWORK_RESTRICTED = 256; // 0x100
field public static final int BLOCKED_REASON_NONE = 0; // 0x0
field @FlaggedApi("com.android.net.flags.blocked_reason_oem_deny_chains") public static final int BLOCKED_REASON_OEM_DENY = 128; // 0x80
field public static final int BLOCKED_REASON_RESTRICTED_MODE = 8; // 0x8
diff --git a/framework/src/android/net/ConnectivityManager.java b/framework/src/android/net/ConnectivityManager.java
index 48ed732..0b37fa5 100644
--- a/framework/src/android/net/ConnectivityManager.java
+++ b/framework/src/android/net/ConnectivityManager.java
@@ -132,6 +132,8 @@
"com.android.net.flags.metered_network_firewall_chains";
static final String BLOCKED_REASON_OEM_DENY_CHAINS =
"com.android.net.flags.blocked_reason_oem_deny_chains";
+ static final String BLOCKED_REASON_NETWORK_RESTRICTED =
+ "com.android.net.flags.blocked_reason_network_restricted";
}
/**
@@ -928,6 +930,17 @@
public static final int BLOCKED_REASON_OEM_DENY = 1 << 7;
/**
+ * Flag to indicate that an app does not have permission to access the specified network,
+ * for example, because it does not have the {@link android.Manifest.permission#INTERNET}
+ * permission.
+ *
+ * @hide
+ */
+ @FlaggedApi(Flags.BLOCKED_REASON_NETWORK_RESTRICTED)
+ @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES)
+ public static final int BLOCKED_REASON_NETWORK_RESTRICTED = 1 << 8;
+
+ /**
* Flag to indicate that an app is subject to Data saver restrictions that would
* result in its metered network access being blocked.
*
@@ -968,6 +981,7 @@
BLOCKED_REASON_LOW_POWER_STANDBY,
BLOCKED_REASON_APP_BACKGROUND,
BLOCKED_REASON_OEM_DENY,
+ BLOCKED_REASON_NETWORK_RESTRICTED,
BLOCKED_METERED_REASON_DATA_SAVER,
BLOCKED_METERED_REASON_USER_RESTRICTED,
BLOCKED_METERED_REASON_ADMIN_DISABLED,
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index cc8eef8..519391f 100755
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -38,6 +38,7 @@
import static android.net.ConnectivityManager.BLOCKED_METERED_REASON_MASK;
import static android.net.ConnectivityManager.BLOCKED_REASON_LOCKDOWN_VPN;
import static android.net.ConnectivityManager.BLOCKED_REASON_NONE;
+import static android.net.ConnectivityManager.BLOCKED_REASON_NETWORK_RESTRICTED;
import static android.net.ConnectivityManager.CALLBACK_IP_CHANGED;
import static android.net.ConnectivityManager.CONNECTIVITY_ACTION;
import static android.net.ConnectivityManager.FIREWALL_CHAIN_BACKGROUND;
@@ -11209,7 +11210,7 @@
final boolean metered = nai.networkCapabilities.isMetered();
final boolean vpnBlocked = isUidBlockedByVpn(nri.mAsUid, mVpnBlockedUidRanges);
callCallbackForRequest(nri, nai, ConnectivityManager.CALLBACK_AVAILABLE,
- getBlockedState(blockedReasons, metered, vpnBlocked));
+ getBlockedState(nri.mAsUid, blockedReasons, metered, vpnBlocked));
}
// Notify the requests on this NAI that the network is now lingered.
@@ -11218,7 +11219,21 @@
notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_LOSING, lingerTime);
}
- private static int getBlockedState(int reasons, boolean metered, boolean vpnBlocked) {
+ private int getPermissionBlockedState(final int uid, final int reasons) {
+ // Before V, the blocked reasons come from NPMS, and that code already behaves as if the
+ // change was disabled: apps without the internet permission will never be told they are
+ // blocked.
+ if (!mDeps.isAtLeastV()) return reasons;
+
+ if (hasInternetPermission(uid)) return reasons;
+
+ return mDeps.isChangeEnabled(NETWORK_BLOCKED_WITHOUT_INTERNET_PERMISSION, uid)
+ ? reasons | BLOCKED_REASON_NETWORK_RESTRICTED
+ : BLOCKED_REASON_NONE;
+ }
+
+ private int getBlockedState(int uid, int reasons, boolean metered, boolean vpnBlocked) {
+ reasons = getPermissionBlockedState(uid, reasons);
if (!metered) reasons &= ~BLOCKED_METERED_REASON_MASK;
return vpnBlocked
? reasons | BLOCKED_REASON_LOCKDOWN_VPN
@@ -11259,8 +11274,10 @@
? isUidBlockedByVpn(nri.mAsUid, newBlockedUidRanges)
: oldVpnBlocked;
- final int oldBlockedState = getBlockedState(blockedReasons, oldMetered, oldVpnBlocked);
- final int newBlockedState = getBlockedState(blockedReasons, newMetered, newVpnBlocked);
+ final int oldBlockedState = getBlockedState(
+ nri.mAsUid, blockedReasons, oldMetered, oldVpnBlocked);
+ final int newBlockedState = getBlockedState(
+ nri.mAsUid, blockedReasons, newMetered, newVpnBlocked);
if (oldBlockedState != newBlockedState) {
callCallbackForRequest(nri, nai, ConnectivityManager.CALLBACK_BLK_CHANGED,
newBlockedState);
@@ -11279,8 +11296,9 @@
final boolean vpnBlocked = isUidBlockedByVpn(uid, mVpnBlockedUidRanges);
final int oldBlockedState = getBlockedState(
- mUidBlockedReasons.get(uid, BLOCKED_REASON_NONE), metered, vpnBlocked);
- final int newBlockedState = getBlockedState(blockedReasons, metered, vpnBlocked);
+ uid, mUidBlockedReasons.get(uid, BLOCKED_REASON_NONE), metered, vpnBlocked);
+ final int newBlockedState =
+ getBlockedState(uid, blockedReasons, metered, vpnBlocked);
if (oldBlockedState == newBlockedState) {
continue;
}
diff --git a/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt b/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
index d2e46af..06bdca6 100644
--- a/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
+++ b/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
@@ -27,6 +27,7 @@
import android.net.ConnectivityManager
import android.net.IDnsResolver
import android.net.INetd
+import android.net.INetd.PERMISSION_INTERNET
import android.net.LinkProperties
import android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL
import android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET
@@ -225,6 +226,9 @@
override fun getSystemProperties() = mock(MockableSystemProperties::class.java)
override fun makeNetIdManager() = TestNetIdManager()
override fun getBpfNetMaps(context: Context?, netd: INetd?) = mock(BpfNetMaps::class.java)
+ .also {
+ doReturn(PERMISSION_INTERNET).`when`(it).getNetPermForUid(anyInt())
+ }
override fun isChangeEnabled(changeId: Long, uid: Int) = true
override fun makeMultinetworkPolicyTracker(
diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index 6e63807..44a8222 100755
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -53,6 +53,7 @@
import static android.net.ConnectivityManager.BLOCKED_METERED_REASON_MASK;
import static android.net.ConnectivityManager.BLOCKED_METERED_REASON_USER_RESTRICTED;
import static android.net.ConnectivityManager.BLOCKED_REASON_BATTERY_SAVER;
+import static android.net.ConnectivityManager.BLOCKED_REASON_DOZE;
import static android.net.ConnectivityManager.BLOCKED_REASON_NONE;
import static android.net.ConnectivityManager.CONNECTIVITY_ACTION;
import static android.net.ConnectivityManager.EXTRA_DEVICE_TYPE;
@@ -9880,6 +9881,28 @@
assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
assertExtraInfoFromCmPresent(mCellAgent);
+ // Remove PERMISSION_INTERNET and disable NETWORK_BLOCKED_WITHOUT_INTERNET_PERMISSION
+ doReturn(INetd.PERMISSION_NONE).when(mBpfNetMaps).getNetPermForUid(Process.myUid());
+ mDeps.setChangeIdEnabled(false,
+ NETWORK_BLOCKED_WITHOUT_INTERNET_PERMISSION, Process.myUid());
+
+ setBlockedReasonChanged(BLOCKED_REASON_DOZE);
+ if (mDeps.isAtLeastV()) {
+ // On V+, network access from app that does not have INTERNET permission is considered
+ // not blocked if NETWORK_BLOCKED_WITHOUT_INTERNET_PERMISSION is disabled.
+ // So blocked status does not change from BLOCKED_REASON_NONE
+ cellNetworkCallback.assertNoCallback();
+ detailedCallback.assertNoCallback();
+ } else {
+ // On U-, onBlockedStatusChanged callback is called with blocked reasons CS receives
+ // from NPMS callback regardless of permission app has.
+ // Note that this cannot actually happen because on U-, NPMS will never notify any
+ // blocked reasons for apps that don't have the INTERNET permission.
+ cellNetworkCallback.expect(BLOCKED_STATUS, mCellAgent, cb -> cb.getBlocked());
+ detailedCallback.expect(BLOCKED_STATUS_INT, mCellAgent,
+ cb -> cb.getReason() == BLOCKED_REASON_DOZE);
+ }
+
mCm.unregisterNetworkCallback(cellNetworkCallback);
}
diff --git a/tests/unit/java/com/android/server/connectivityservice/CSBlockedReasonsTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSBlockedReasonsTest.kt
index 2694916..0590fbb 100644
--- a/tests/unit/java/com/android/server/connectivityservice/CSBlockedReasonsTest.kt
+++ b/tests/unit/java/com/android/server/connectivityservice/CSBlockedReasonsTest.kt
@@ -20,6 +20,7 @@
import android.net.ConnectivityManager.BLOCKED_METERED_REASON_USER_RESTRICTED
import android.net.ConnectivityManager.BLOCKED_REASON_APP_BACKGROUND
import android.net.ConnectivityManager.BLOCKED_REASON_DOZE
+import android.net.ConnectivityManager.BLOCKED_REASON_NETWORK_RESTRICTED
import android.net.ConnectivityManager.BLOCKED_REASON_NONE
import android.net.ConnectivityManager.FIREWALL_CHAIN_BACKGROUND
import android.net.ConnectivityManager.FIREWALL_CHAIN_DOZABLE
@@ -27,6 +28,7 @@
import android.net.ConnectivityManager.FIREWALL_RULE_ALLOW
import android.net.ConnectivityManager.FIREWALL_RULE_DENY
import android.net.ConnectivitySettingsManager
+import android.net.INetd.PERMISSION_NONE
import android.net.Network
import android.net.NetworkCapabilities
import android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET
@@ -36,6 +38,7 @@
import android.net.NetworkCapabilities.TRANSPORT_CELLULAR
import android.net.NetworkCapabilities.TRANSPORT_WIFI
import android.net.NetworkRequest
+import android.net.connectivity.ConnectivityCompatChanges.NETWORK_BLOCKED_WITHOUT_INTERNET_PERMISSION
import android.os.Build
import android.os.Process
import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo
@@ -144,6 +147,12 @@
cm.setDataSaverEnabled(false)
cellCb.expectBlockedStatusChanged(cellAgent.network, BLOCKED_REASON_APP_BACKGROUND)
+ // waitForIdle since stubbing bpfNetMaps while CS handler thread calls
+ // bpfNetMaps.getNetPermForUid throws exception.
+ // The expectBlockedStatusChanged just above guarantees that the onBlockedStatusChanged
+ // method on this callback was called, but it does not guarantee that ConnectivityService
+ // has finished processing all onBlockedStatusChanged callbacks for all requests.
+ waitForIdle()
// Enable data saver
doReturn(BLOCKED_REASON_APP_BACKGROUND or BLOCKED_METERED_REASON_DATA_SAVER)
.`when`(bpfNetMaps).getUidNetworkingBlockedReasons(Process.myUid())
@@ -186,6 +195,12 @@
blockedReason = BLOCKED_REASON_DOZE
)
+ // waitForIdle since stubbing bpfNetMaps while CS handler thread calls
+ // bpfNetMaps.getNetPermForUid throws exception.
+ // The expectBlockedStatusChanged just above guarantees that the onBlockedStatusChanged
+ // method on this callback was called, but it does not guarantee that ConnectivityService
+ // has finished processing all onBlockedStatusChanged callbacks for all requests.
+ waitForIdle()
// Set RULE_ALLOW on metered deny chain
doReturn(BLOCKED_REASON_DOZE)
.`when`(bpfNetMaps).getUidNetworkingBlockedReasons(Process.myUid())
@@ -343,4 +358,61 @@
cm.unregisterNetworkCallback(wifiCb)
cm.unregisterNetworkCallback(cb)
}
+
+ private fun doTestBlockedReasonsNoInternetPermission(blockedByNoInternetPermission: Boolean) {
+ doReturn(PERMISSION_NONE).`when`(bpfNetMaps).getNetPermForUid(Process.myUid())
+
+ val wifiCb = DetailedBlockedStatusCallback()
+ cm.requestNetwork(wifiRequest(), wifiCb)
+ val wifiAgent = Agent(nc = wifiNc())
+ wifiAgent.connect()
+ val expectedBlockedReason = if (blockedByNoInternetPermission) {
+ BLOCKED_REASON_NETWORK_RESTRICTED
+ } else {
+ BLOCKED_REASON_NONE
+ }
+ wifiCb.expectAvailableCallbacks(
+ wifiAgent.network,
+ validated = false,
+ blockedReason = expectedBlockedReason
+ )
+
+ // Enable background firewall chain
+ doReturn(BLOCKED_REASON_APP_BACKGROUND)
+ .`when`(bpfNetMaps).getUidNetworkingBlockedReasons(Process.myUid())
+ cm.setFirewallChainEnabled(FIREWALL_CHAIN_BACKGROUND, true)
+ if (blockedByNoInternetPermission) {
+ wifiCb.expectBlockedStatusChanged(
+ wifiAgent.network,
+ BLOCKED_REASON_NETWORK_RESTRICTED or BLOCKED_REASON_APP_BACKGROUND
+ )
+ }
+
+ // Disable background firewall chain
+ doReturn(BLOCKED_REASON_NONE)
+ .`when`(bpfNetMaps).getUidNetworkingBlockedReasons(Process.myUid())
+ cm.setFirewallChainEnabled(FIREWALL_CHAIN_BACKGROUND, false)
+ if (blockedByNoInternetPermission) {
+ wifiCb.expectBlockedStatusChanged(
+ wifiAgent.network,
+ BLOCKED_REASON_NETWORK_RESTRICTED
+ )
+ } else {
+ // No callback is expected since blocked reasons does not change from
+ // BLOCKED_REASON_NONE.
+ wifiCb.assertNoCallback()
+ }
+ }
+
+ @Test
+ fun testBlockedReasonsNoInternetPermission_changeDisabled() {
+ deps.setChangeIdEnabled(false, NETWORK_BLOCKED_WITHOUT_INTERNET_PERMISSION)
+ doTestBlockedReasonsNoInternetPermission(blockedByNoInternetPermission = false)
+ }
+
+ @Test
+ fun testBlockedReasonsNoInternetPermission_changeEnabled() {
+ deps.setChangeIdEnabled(true, NETWORK_BLOCKED_WITHOUT_INTERNET_PERMISSION)
+ doTestBlockedReasonsNoInternetPermission(blockedByNoInternetPermission = true)
+ }
}
diff --git a/tests/unit/java/com/android/server/connectivityservice/CSIngressDiscardRuleTests.kt b/tests/unit/java/com/android/server/connectivityservice/CSIngressDiscardRuleTests.kt
index bb7fb51..93f6e81 100644
--- a/tests/unit/java/com/android/server/connectivityservice/CSIngressDiscardRuleTests.kt
+++ b/tests/unit/java/com/android/server/connectivityservice/CSIngressDiscardRuleTests.kt
@@ -37,6 +37,7 @@
import com.android.testutils.TestableNetworkCallback
import org.junit.Test
import org.junit.runner.RunWith
+import org.mockito.InOrder
import org.mockito.Mockito.inOrder
import org.mockito.Mockito.never
import org.mockito.Mockito.timeout
@@ -96,6 +97,11 @@
private val LOCAL_IPV6_ADDRRESS = InetAddresses.parseNumericAddress("fe80::1234")
private val LOCAL_IPV6_LINK_ADDRRESS = LinkAddress(LOCAL_IPV6_ADDRRESS, 64)
+ fun verifyNoMoreIngressDiscardRuleChange(inorder: InOrder) {
+ inorder.verify(bpfNetMaps, never()).setIngressDiscardRule(any(), any())
+ inorder.verify(bpfNetMaps, never()).removeIngressDiscardRule(any())
+ }
+
@Test
fun testVpnIngressDiscardRule_UpdateVpnAddress() {
// non-VPN network whose address will be not duplicated with VPN address
@@ -148,7 +154,7 @@
// IngressDiscardRule is added to the VPN address
inorder.verify(bpfNetMaps).setIngressDiscardRule(IPV6_ADDRESS, VPN_IFNAME)
- inorder.verifyNoMoreInteractions()
+ verifyNoMoreIngressDiscardRuleChange(inorder)
// The VPN interface name is changed
val newlp = lp(VPN_IFNAME2, IPV6_LINK_ADDRESS, LOCAL_IPV6_LINK_ADDRRESS)
@@ -157,7 +163,7 @@
// IngressDiscardRule is updated with the new interface name
inorder.verify(bpfNetMaps).setIngressDiscardRule(IPV6_ADDRESS, VPN_IFNAME2)
- inorder.verifyNoMoreInteractions()
+ verifyNoMoreIngressDiscardRuleChange(inorder)
agent.disconnect()
inorder.verify(bpfNetMaps, timeout(TIMEOUT_MS)).removeIngressDiscardRule(IPV6_ADDRESS)
@@ -206,10 +212,10 @@
// IngressDiscardRule for IPV6_ADDRESS2 is removed but IngressDiscardRule for
// IPV6_LINK_ADDRESS is not added since Wi-Fi also uses IPV6_LINK_ADDRESS
inorder.verify(bpfNetMaps).removeIngressDiscardRule(IPV6_ADDRESS2)
- inorder.verifyNoMoreInteractions()
+ verifyNoMoreIngressDiscardRuleChange(inorder)
vpnAgent.disconnect()
- inorder.verifyNoMoreInteractions()
+ verifyNoMoreIngressDiscardRuleChange(inorder)
cm.unregisterNetworkCallback(cb)
}
@@ -225,7 +231,7 @@
// IngressDiscardRule is added to the VPN address
inorder.verify(bpfNetMaps).setIngressDiscardRule(IPV6_ADDRESS, VPN_IFNAME)
- inorder.verifyNoMoreInteractions()
+ verifyNoMoreIngressDiscardRuleChange(inorder)
val nr = nr(TRANSPORT_WIFI)
val cb = TestableNetworkCallback()
@@ -247,7 +253,7 @@
// IngressDiscardRule is added to the VPN address since the VPN address is not duplicated
// with the Wi-Fi address
inorder.verify(bpfNetMaps).setIngressDiscardRule(IPV6_ADDRESS, VPN_IFNAME)
- inorder.verifyNoMoreInteractions()
+ verifyNoMoreIngressDiscardRuleChange(inorder)
// The Wi-Fi address is changed back to the same address as the VPN interface
wifiAgent.sendLinkProperties(wifiLp)
diff --git a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt b/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt
index 63ef86e..99a8a3d 100644
--- a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt
+++ b/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt
@@ -27,6 +27,7 @@
import android.content.res.Resources
import android.net.ConnectivityManager
import android.net.INetd
+import android.net.INetd.PERMISSION_INTERNET
import android.net.InetAddresses
import android.net.LinkProperties
import android.net.LocalNetworkConfig
@@ -89,6 +90,7 @@
import org.junit.Rule
import org.junit.rules.TestName
import org.mockito.AdditionalAnswers.delegatesTo
+import org.mockito.ArgumentMatchers.anyInt
import org.mockito.Mockito.doAnswer
import org.mockito.Mockito.doReturn
import org.mockito.Mockito.mock
@@ -187,7 +189,9 @@
val connResources = makeMockConnResources(sysResources, packageManager)
val netd = mock<INetd>()
- val bpfNetMaps = mock<BpfNetMaps>()
+ val bpfNetMaps = mock<BpfNetMaps>().also {
+ doReturn(PERMISSION_INTERNET).`when`(it).getNetPermForUid(anyInt())
+ }
val clatCoordinator = mock<ClatCoordinator>()
val networkRequestStateStatsMetrics = mock<NetworkRequestStateStatsMetrics>()
val proxyTracker = ProxyTracker(