diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index e7320014b5..195f0466d3 100755 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -368,7 +368,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private static final int DEFAULT_NASCENT_DELAY_MS = 5_000; // The maximum value for the blocking validation result, in milliseconds. - public static final int MAX_VALIDATION_FAILURE_BLOCKING_TIME_MS = 10000; + public static final int MAX_VALIDATION_IGNORE_AFTER_ROAM_TIME_MS = 10000; // The maximum number of network request allowed per uid before an exception is thrown. @VisibleForTesting @@ -4241,12 +4241,18 @@ public class ConnectivityService extends IConnectivityManager.Stub return nai.isCreated() && !nai.isDestroyed(); } - private boolean shouldIgnoreValidationFailureAfterRoam(NetworkAgentInfo nai) { + @VisibleForTesting + boolean shouldIgnoreValidationFailureAfterRoam(NetworkAgentInfo nai) { // T+ devices should use unregisterAfterReplacement. if (SdkLevel.isAtLeastT()) return false; + + // If the network never roamed, return false. The check below is not sufficient if time + // since boot is less than blockTimeOut, though that's extremely unlikely to happen. + if (nai.lastRoamTime == 0) return false; + final long blockTimeOut = Long.valueOf(mResources.get().getInteger( R.integer.config_validationFailureAfterRoamIgnoreTimeMillis)); - if (blockTimeOut <= MAX_VALIDATION_FAILURE_BLOCKING_TIME_MS + if (blockTimeOut <= MAX_VALIDATION_IGNORE_AFTER_ROAM_TIME_MS && blockTimeOut >= 0) { final long currentTimeMs = SystemClock.elapsedRealtime(); long timeSinceLastRoam = currentTimeMs - nai.lastRoamTime; @@ -9774,8 +9780,8 @@ public class ConnectivityService extends IConnectivityManager.Stub return ((VpnTransportInfo) ti).getType(); } - private void maybeUpdateWifiRoamTimestamp(NetworkAgentInfo nai, NetworkCapabilities nc) { - if (nai == null) return; + private void maybeUpdateWifiRoamTimestamp(@NonNull NetworkAgentInfo nai, + @NonNull NetworkCapabilities nc) { final TransportInfo prevInfo = nai.networkCapabilities.getTransportInfo(); final TransportInfo newInfo = nc.getTransportInfo(); if (!(prevInfo instanceof WifiInfo) || !(newInfo instanceof WifiInfo)) { diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index bc16920261..2d8cf80246 100755 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -16782,23 +16782,25 @@ public class ConnectivityServiceTest { assertThrows(NullPointerException.class, () -> mService.unofferNetwork(null)); } - public void doTestIgnoreValidationAfterRoam(final boolean enabled) throws Exception { - assumeFalse(SdkLevel.isAtLeastT()); - doReturn(enabled ? 5000 : -1).when(mResources) + public void doTestIgnoreValidationAfterRoam(int resValue, final boolean enabled) + throws Exception { + doReturn(resValue).when(mResources) .getInteger(R.integer.config_validationFailureAfterRoamIgnoreTimeMillis); + final String bssid1 = "AA:AA:AA:AA:AA:AA"; + final String bssid2 = "BB:BB:BB:BB:BB:BB"; mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); mCellNetworkAgent.connect(true); NetworkCapabilities wifiNc1 = new NetworkCapabilities() .addCapability(NET_CAPABILITY_INTERNET) + .addCapability(NET_CAPABILITY_NOT_VPN) + .addCapability(NET_CAPABILITY_NOT_RESTRICTED) + .addCapability(NET_CAPABILITY_TRUSTED) .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED) .addTransportType(TRANSPORT_WIFI) - .setTransportInfo(new WifiInfo.Builder().setBssid("AA:AA:AA:AA:AA:AA").build()); - NetworkCapabilities wifiNc2 = new NetworkCapabilities() - .addCapability(NET_CAPABILITY_INTERNET) - .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED) - .addTransportType(TRANSPORT_WIFI) - .setTransportInfo(new WifiInfo.Builder().setBssid("BB:BB:BB:BB:BB:BB").build()); + .setTransportInfo(new WifiInfo.Builder().setBssid(bssid1).build()); + NetworkCapabilities wifiNc2 = new NetworkCapabilities(wifiNc1) + .setTransportInfo(new WifiInfo.Builder().setBssid(bssid2).build()); final LinkProperties wifiLp = new LinkProperties(); wifiLp.setInterfaceName(WIFI_IFNAME); mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI, wifiLp, wifiNc1); @@ -16862,6 +16864,7 @@ public class ConnectivityServiceTest { wifiNetworkCallback.expectCapabilitiesWithout(NET_CAPABILITY_PARTIAL_CONNECTIVITY, mWiFiNetworkAgent); } + mCm.unregisterNetworkCallback(wifiNetworkCallback); // Wi-Fi roams from wifiNc1 to wifiNc2, and now becomes really invalid. If validation // failures after roam are not ignored, this will cause cell to become the default network. @@ -16872,39 +16875,76 @@ public class ConnectivityServiceTest { mWiFiNetworkAgent.setNetworkInvalid(false /* isStrictMode */); mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), false); - if (!enabled) { + if (enabled) { + // Network validation failed, but the result will be ignored. + assertTrue(mCm.getNetworkCapabilities(mWiFiNetworkAgent.getNetwork()).hasCapability( + NET_CAPABILITY_VALIDATED)); + mWiFiNetworkAgent.setNetworkValid(false); + + // Behavior of after config_validationFailureAfterRoamIgnoreTimeMillis + ConditionVariable waitForValidationBlock = new ConditionVariable(); + doReturn(50).when(mResources) + .getInteger(R.integer.config_validationFailureAfterRoamIgnoreTimeMillis); + // Wi-Fi roaming from wifiNc2 to wifiNc1. + mWiFiNetworkAgent.setNetworkCapabilities(wifiNc1, true); + mWiFiNetworkAgent.setNetworkInvalid(false); + waitForValidationBlock.block(150); + mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), false); + mDefaultNetworkCallback.expectAvailableCallbacksValidated(mCellNetworkAgent); + } else { mDefaultNetworkCallback.expectAvailableCallbacksValidated(mCellNetworkAgent); - return; } - // Network validation failed, but the result will be ignored. - assertTrue(mCm.getNetworkCapabilities(mWiFiNetworkAgent.getNetwork()).hasCapability( - NET_CAPABILITY_VALIDATED)); - mWiFiNetworkAgent.setNetworkValid(false); + // Wi-Fi is still connected and would become the default network if cell were to + // disconnect. This assertion ensures that the switch to cellular was not caused by + // Wi-Fi disconnecting (e.g., because the capability change to wifiNc2 caused it + // to stop satisfying the default request). + mCellNetworkAgent.disconnect(); + mDefaultNetworkCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent); + mDefaultNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); - // Behavior of after config_validationFailureAfterRoamIgnoreTimeMillis - ConditionVariable waitForValidationBlock = new ConditionVariable(); - doReturn(50).when(mResources) - .getInteger(R.integer.config_validationFailureAfterRoamIgnoreTimeMillis); - // Wi-Fi roaming from wifiNc2 to wifiNc1. - mWiFiNetworkAgent.setNetworkCapabilities(wifiNc1, true); - mWiFiNetworkAgent.setNetworkInvalid(false); - waitForValidationBlock.block(150); - mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), false); - mDefaultNetworkCallback.expectAvailableCallbacksValidated(mCellNetworkAgent); - - mCm.unregisterNetworkCallback(wifiNetworkCallback); } @Test public void testIgnoreValidationAfterRoamDisabled() throws Exception { - doTestIgnoreValidationAfterRoam(false /* enabled */); + doTestIgnoreValidationAfterRoam(-1, false /* enabled */); } + @Test public void testIgnoreValidationAfterRoamEnabled() throws Exception { - doTestIgnoreValidationAfterRoam(true /* enabled */); + final boolean enabled = !SdkLevel.isAtLeastT(); + doTestIgnoreValidationAfterRoam(5_000, enabled); } + @Test + public void testShouldIgnoreValidationFailureAfterRoam() { + // Always disabled on T+. + assumeFalse(SdkLevel.isAtLeastT()); + + NetworkAgentInfo nai = fakeWifiNai(new NetworkCapabilities()); + + // Enabled, but never roamed. + doReturn(5_000).when(mResources) + .getInteger(R.integer.config_validationFailureAfterRoamIgnoreTimeMillis); + assertEquals(0, nai.lastRoamTime); + assertFalse(mService.shouldIgnoreValidationFailureAfterRoam(nai)); + + // Roamed recently. + nai.lastRoamTime = SystemClock.elapsedRealtime() - 500 /* ms */; + assertTrue(mService.shouldIgnoreValidationFailureAfterRoam(nai)); + + // Disabled due to invalid setting (maximum is 10 seconds). + doReturn(15_000).when(mResources) + .getInteger(R.integer.config_validationFailureAfterRoamIgnoreTimeMillis); + assertFalse(mService.shouldIgnoreValidationFailureAfterRoam(nai)); + + // Disabled. + doReturn(-1).when(mResources) + .getInteger(R.integer.config_validationFailureAfterRoamIgnoreTimeMillis); + assertFalse(mService.shouldIgnoreValidationFailureAfterRoam(nai)); + } + + @Test public void testLegacyTetheringApiGuardWithProperPermission() throws Exception { final String testIface = "test0";