commit 73be9a093380643934a604bf907895348b29ed67
author Chiachang Wang <chiachangwang@google.com> Wed Mar 13 13:19:04 2024 +0000
committer Chiachang Wang <chiachangwang@google.com> Thu Mar 14 04:32:11 2024 +0000
tree f626aff940707dd3a5571a52e31e39259e16b9c6
parent fee93b0528f08acb5fe32972d12dc6adb8ab9595

Ignore no permission calls for CaptivePortal.appRequest

This enforceNetworkStackPermission() should be adopted to check
the required permission but this may be break OEM captive portal
apps. Simply ignore the request if the caller does not have
permission.

This commit also fixes the format issues that is auto-fixed by
ktlint.

Fix: 329385091
Test: ates FrameworksNetTests
Change-Id: Ia973a1078eda3134fc8ff261d3954cd050767880

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

diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index 30b14b2..fe3bbbd 100755
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -6003,7 +6003,15 @@
             if (nm == null) return;
 
             if (request == CaptivePortal.APP_REQUEST_REEVALUATION_REQUIRED) {
-                enforceNetworkStackPermission(mContext);
+                // This enforceNetworkStackPermission() should be adopted to check
+                // the required permission but this may be break OEM captive portal
+                // apps. Simply ignore the request if the caller does not have
+                // permission.
+                if (!hasNetworkStackPermission()) {
+                    Log.e(TAG, "Calling appRequest() without proper permission. Skip");
+                    return;
+                }
+
                 nm.forceReevaluation(mDeps.getCallingUid());
             }
         }

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

diff --git a/tests/unit/java/com/android/server/connectivityservice/CSCaptivePortalAppTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSCaptivePortalAppTest.kt
index be2b29c..0bad60d 100644
--- a/tests/unit/java/com/android/server/connectivityservice/CSCaptivePortalAppTest.kt
+++ b/tests/unit/java/com/android/server/connectivityservice/CSCaptivePortalAppTest.kt
@@ -20,6 +20,7 @@
 import android.content.Intent
 import android.content.pm.PackageManager.PERMISSION_DENIED
 import android.content.pm.PackageManager.PERMISSION_GRANTED
+import android.net.CaptivePortal
 import android.net.ConnectivityManager.ACTION_CAPTIVE_PORTAL_SIGN_IN
 import android.net.ConnectivityManager.EXTRA_CAPTIVE_PORTAL
 import android.net.IpPrefix
@@ -33,23 +34,23 @@
 import android.net.NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED
 import android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED
 import android.net.NetworkCapabilities.TRANSPORT_WIFI
-import android.net.NetworkStack
-import android.net.CaptivePortal
 import android.net.NetworkRequest
 import android.net.NetworkScore
 import android.net.NetworkScore.KEEP_CONNECTED_FOR_TEST
+import android.net.NetworkStack
 import android.net.RouteInfo
 import android.os.Build
 import android.os.Bundle
 import androidx.test.filters.SmallTest
 import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo
 import com.android.testutils.DevSdkIgnoreRunner
-import com.android.testutils.assertThrows
 import com.android.testutils.TestableNetworkCallback
+import kotlin.test.assertEquals
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.ArgumentMatchers.anyInt
+import org.mockito.Mockito.never
 import org.mockito.Mockito.verify
-import kotlin.test.assertEquals
 
 // This allows keeping all the networks connected without having to file individual requests
 // for them.
@@ -95,16 +96,22 @@
         captivePortalCallback.expectAvailableCallbacksUnvalidated(wifiAgent)
         val signInIntent = startCaptivePortalApp(wifiAgent)
         // Remove the granted permissions
-        context.setPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK,
-                PERMISSION_DENIED)
+        context.setPermission(
+            NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK,
+                PERMISSION_DENIED
+        )
         context.setPermission(NETWORK_STACK, PERMISSION_DENIED)
         val captivePortal: CaptivePortal? = signInIntent.getParcelableExtra(EXTRA_CAPTIVE_PORTAL)
-        assertThrows(SecurityException::class.java, { captivePortal?.reevaluateNetwork() })
+        captivePortal?.reevaluateNetwork()
+        verify(wifiAgent.networkMonitor, never()).forceReevaluation(anyInt())
     }
 
     private fun createWifiAgent(): CSAgentWrapper {
-        return Agent(score = keepScore(), lp = lp(WIFI_IFACE),
-                nc = nc(TRANSPORT_WIFI, NET_CAPABILITY_INTERNET))
+        return Agent(
+            score = keepScore(),
+            lp = lp(WIFI_IFACE),
+                nc = nc(TRANSPORT_WIFI, NET_CAPABILITY_INTERNET)
+        )
     }
 
     private fun startCaptivePortalApp(networkAgent: CSAgentWrapper): Intent {