From ff55aeb9166f89a3ad22bc10287d4e5c0378b606 Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Wed, 16 Jun 2021 11:37:53 +0000 Subject: [PATCH 1/3] Remove ConnectivityServiceTest signature perms use To allow unit tests to run without platform certificates, remove signature permission usage in ConnectivityServiceTest. This mocks permission checks done in ConnectivityService for which the test assumed that the permission was held, and mocks calls to BatteryStatsManager. Calls to ActivityManagerService (through PendingIntent) are done with shell permissions as the test uses real PendingIntent mechanics. Bug: 187935317 Test: atest FrameworksNetTests Merged-In: If309d653ac2e9bbcf1b94bcee6336367289df359 Change-Id: If309d653ac2e9bbcf1b94bcee6336367289df359 Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/1736615 (cherry picked from commit 595dda36040c1a7a778ce97d2484324a1162fe83) Change-Id: Idb19b0f7cb87bb4d9de7a0b1a0e4128c2d6b9c6d --- .../android/server/ConnectivityService.java | 18 +++- tests/unit/Android.bp | 1 - .../server/ConnectivityServiceTest.java | 86 +++++++++++++++---- .../server/net/NetworkStatsServiceTest.java | 26 ++++++ 4 files changed, 112 insertions(+), 19 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 899286b819..f61eee6de3 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -1276,6 +1276,20 @@ public class ConnectivityService extends IConnectivityManager.Stub public boolean getCellular464XlatEnabled() { return NetworkProperties.isCellular464XlatEnabled().orElse(true); } + + /** + * @see PendingIntent#intentFilterEquals + */ + public boolean intentFilterEquals(PendingIntent a, PendingIntent b) { + return a.intentFilterEquals(b); + } + + /** + * @see LocationPermissionChecker + */ + public LocationPermissionChecker makeLocationPermissionChecker(Context context) { + return new LocationPermissionChecker(context); + } } public ConnectivityService(Context context) { @@ -1343,7 +1357,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mNetd = netd; mTelephonyManager = (TelephonyManager) mContext.getSystemService(Context.TELEPHONY_SERVICE); mAppOpsManager = (AppOpsManager) mContext.getSystemService(Context.APP_OPS_SERVICE); - mLocationPermissionChecker = new LocationPermissionChecker(mContext); + mLocationPermissionChecker = mDeps.makeLocationPermissionChecker(mContext); // To ensure uid state is synchronized with Network Policy, register for // NetworkPolicyManagerService events must happen prior to NetworkPolicyManagerService @@ -3934,7 +3948,7 @@ public class ConnectivityService extends IConnectivityManager.Stub for (Map.Entry entry : mNetworkRequests.entrySet()) { PendingIntent existingPendingIntent = entry.getValue().mPendingIntent; if (existingPendingIntent != null && - existingPendingIntent.intentFilterEquals(pendingIntent)) { + mDeps.intentFilterEquals(existingPendingIntent, pendingIntent)) { return entry.getValue(); } } diff --git a/tests/unit/Android.bp b/tests/unit/Android.bp index beae0cf7a3..ba542733d6 100644 --- a/tests/unit/Android.bp +++ b/tests/unit/Android.bp @@ -60,7 +60,6 @@ android_test { "java/**/*.kt", ], test_suites: ["device-tests"], - certificate: "platform", jarjar_rules: "jarjar-rules.txt", static_libs: [ "androidx.test.rules", diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index be7239d3f9..dd1e888e01 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -18,10 +18,15 @@ package com.android.server; import static android.Manifest.permission.CHANGE_NETWORK_STATE; import static android.Manifest.permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS; +import static android.Manifest.permission.CONTROL_OEM_PAID_NETWORK_PREFERENCE; +import static android.Manifest.permission.CREATE_USERS; import static android.Manifest.permission.DUMP; +import static android.Manifest.permission.GET_INTENT_SENDER_INTENT; import static android.Manifest.permission.LOCAL_MAC_ADDRESS; import static android.Manifest.permission.NETWORK_FACTORY; import static android.Manifest.permission.NETWORK_SETTINGS; +import static android.Manifest.permission.NETWORK_STACK; +import static android.Manifest.permission.PACKET_KEEPALIVE_OFFLOAD; import static android.app.PendingIntent.FLAG_IMMUTABLE; import static android.content.Intent.ACTION_PACKAGE_ADDED; import static android.content.Intent.ACTION_PACKAGE_REMOVED; @@ -134,6 +139,7 @@ import static com.android.testutils.MiscAsserts.assertLength; import static com.android.testutils.MiscAsserts.assertRunsInAtMost; import static com.android.testutils.MiscAsserts.assertSameElements; import static com.android.testutils.MiscAsserts.assertThrows; +import static com.android.testutils.TestPermissionUtil.runAsShell; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -259,6 +265,7 @@ import android.net.shared.NetworkMonitorUtils; import android.net.shared.PrivateDnsConfig; import android.net.util.MultinetworkPolicyTracker; import android.os.BadParcelableException; +import android.os.BatteryStatsManager; import android.os.Binder; import android.os.Build; import android.os.Bundle; @@ -297,6 +304,7 @@ import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; import com.android.connectivity.resources.R; +import com.android.internal.app.IBatteryStats; import com.android.internal.net.VpnConfig; import com.android.internal.net.VpnProfile; import com.android.internal.util.ArrayUtils; @@ -305,6 +313,7 @@ import com.android.internal.util.test.BroadcastInterceptingContext; import com.android.internal.util.test.FakeSettingsProvider; import com.android.net.module.util.ArrayTrackRecord; import com.android.net.module.util.CollectionUtils; +import com.android.net.module.util.LocationPermissionChecker; import com.android.server.ConnectivityService.ConnectivityDiagnosticsCallbackInfo; import com.android.server.ConnectivityService.NetworkRequestInfo; import com.android.server.connectivity.MockableSystemProperties; @@ -488,6 +497,11 @@ public class ConnectivityServiceTest { @Mock Resources mResources; @Mock ProxyTracker mProxyTracker; + // BatteryStatsManager is final and cannot be mocked with regular mockito, so just mock the + // underlying binder calls. + final BatteryStatsManager mBatteryStatsManager = + new BatteryStatsManager(mock(IBatteryStats.class)); + private ArgumentCaptor mResolverParamsParcelCaptor = ArgumentCaptor.forClass(ResolverParamsParcel.class); @@ -579,6 +593,7 @@ public class ConnectivityServiceTest { if (Context.NETWORK_POLICY_SERVICE.equals(name)) return mNetworkPolicyManager; if (Context.SYSTEM_CONFIG_SERVICE.equals(name)) return mSystemConfigManager; if (Context.NETWORK_STATS_SERVICE.equals(name)) return mStatsManager; + if (Context.BATTERY_STATS_SERVICE.equals(name)) return mBatteryStatsManager; return super.getSystemService(name); } @@ -659,6 +674,15 @@ public class ConnectivityServiceTest { public void setPermission(String permission, Integer granted) { mMockedPermissions.put(permission, granted); } + + @Override + public Intent registerReceiverForAllUsers(@Nullable BroadcastReceiver receiver, + @NonNull IntentFilter filter, @Nullable String broadcastPermission, + @Nullable Handler scheduler) { + // TODO: ensure MultinetworkPolicyTracker's BroadcastReceiver is tested; just returning + // null should not pass the test + return null; + } } private void waitForIdle() { @@ -1208,7 +1232,24 @@ public class ConnectivityServiceTest { return mDeviceIdleInternal; } }, - mNetworkManagementService, mMockNetd, userId, mVpnProfileStore); + mNetworkManagementService, mMockNetd, userId, mVpnProfileStore, + new SystemServices(mServiceContext) { + @Override + public String settingsSecureGetStringForUser(String key, int userId) { + switch (key) { + // Settings keys not marked as @Readable are not readable from + // non-privileged apps, unless marked as testOnly=true + // (atest refuses to install testOnly=true apps), even if mocked + // in the content provider, because + // Settings.Secure.NameValueCache#getStringForUser checks the key + // before querying the mock settings provider. + case Settings.Secure.ALWAYS_ON_VPN_APP: + return null; + default: + return super.settingsSecureGetStringForUser(key, userId); + } + } + }, new Ikev2SessionCreator()); } public void setUids(Set uids) { @@ -1592,6 +1633,11 @@ public class ConnectivityServiceTest { mServiceContext = new MockContext(InstrumentationRegistry.getContext(), new FakeSettingsProvider()); mServiceContext.setUseRegisteredHandlers(true); + mServiceContext.setPermission(NETWORK_FACTORY, PERMISSION_GRANTED); + mServiceContext.setPermission(NETWORK_STACK, PERMISSION_GRANTED); + mServiceContext.setPermission(CONTROL_OEM_PAID_NETWORK_PREFERENCE, PERMISSION_GRANTED); + mServiceContext.setPermission(PACKET_KEEPALIVE_OFFLOAD, PERMISSION_GRANTED); + mServiceContext.setPermission(CONNECTIVITY_USE_RESTRICTED_NETWORKS, PERMISSION_GRANTED); mAlarmManagerThread = new HandlerThread("TestAlarmManager"); mAlarmManagerThread.start(); @@ -1651,6 +1697,13 @@ public class ConnectivityServiceTest { return mPolicyTracker; }).when(deps).makeMultinetworkPolicyTracker(any(), any(), any()); doReturn(true).when(deps).getCellular464XlatEnabled(); + doAnswer(inv -> + new LocationPermissionChecker(inv.getArgument(0)) { + @Override + protected int getCurrentUser() { + return runAsShell(CREATE_USERS, () -> super.getCurrentUser()); + } + }).when(deps).makeLocationPermissionChecker(any()); doReturn(60000).when(mResources).getInteger(R.integer.config_networkTransitionTimeout); doReturn("").when(mResources).getString(R.string.config_networkCaptivePortalServerUrl); @@ -1680,6 +1733,12 @@ public class ConnectivityServiceTest { doReturn(mResources).when(mockResContext).getResources(); ConnectivityResources.setResourcesContextForTest(mockResContext); + doAnswer(inv -> { + final PendingIntent a = inv.getArgument(0); + final PendingIntent b = inv.getArgument(1); + return runAsShell(GET_INTENT_SENDER_INTENT, () -> a.intentFilterEquals(b)); + }).when(deps).intentFilterEquals(any(), any()); + return deps; } @@ -9357,8 +9416,7 @@ public class ConnectivityServiceTest { mServiceContext.setPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, PERMISSION_DENIED); mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_DENIED); - mServiceContext.setPermission(Manifest.permission.NETWORK_STACK, - PERMISSION_DENIED); + mServiceContext.setPermission(NETWORK_STACK, PERMISSION_DENIED); mServiceContext.setPermission(Manifest.permission.NETWORK_SETUP_WIZARD, PERMISSION_DENIED); } @@ -9799,7 +9857,7 @@ public class ConnectivityServiceTest { setupConnectionOwnerUid(vpnOwnerUid, vpnType); // Test as VPN app - mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + mServiceContext.setPermission(NETWORK_STACK, PERMISSION_DENIED); mServiceContext.setPermission( NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, PERMISSION_DENIED); } @@ -9839,8 +9897,7 @@ public class ConnectivityServiceTest { public void testGetConnectionOwnerUidVpnServiceNetworkStackDoesNotThrow() throws Exception { final int myUid = Process.myUid(); setupConnectionOwnerUid(myUid, VpnManager.TYPE_VPN_SERVICE); - mServiceContext.setPermission( - android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); + mServiceContext.setPermission(NETWORK_STACK, PERMISSION_GRANTED); assertEquals(42, mService.getConnectionOwnerUid(getTestConnectionInfo())); } @@ -10008,8 +10065,7 @@ public class ConnectivityServiceTest { public void testCheckConnectivityDiagnosticsPermissionsNetworkStack() throws Exception { final NetworkAgentInfo naiWithoutUid = fakeMobileNai(new NetworkCapabilities()); - mServiceContext.setPermission( - android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); + mServiceContext.setPermission(NETWORK_STACK, PERMISSION_GRANTED); assertTrue( "NetworkStack permission not applied", mService.checkConnectivityDiagnosticsPermissions( @@ -10025,7 +10081,7 @@ public class ConnectivityServiceTest { nc.setAdministratorUids(new int[] {wrongUid}); final NetworkAgentInfo naiWithUid = fakeWifiNai(nc); - mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + mServiceContext.setPermission(NETWORK_STACK, PERMISSION_DENIED); assertFalse( "Mismatched uid/package name should not pass the location permission check", @@ -10035,7 +10091,7 @@ public class ConnectivityServiceTest { private void verifyConnectivityDiagnosticsPermissionsWithNetworkAgentInfo( NetworkAgentInfo info, boolean expectPermission) { - mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + mServiceContext.setPermission(NETWORK_STACK, PERMISSION_DENIED); assertEquals( "Unexpected ConnDiags permission", @@ -10103,7 +10159,7 @@ public class ConnectivityServiceTest { setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION); - mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + mServiceContext.setPermission(NETWORK_STACK, PERMISSION_DENIED); assertTrue( "NetworkCapabilities administrator uid permission not applied", @@ -10120,7 +10176,7 @@ public class ConnectivityServiceTest { setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION); - mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + mServiceContext.setPermission(NETWORK_STACK, PERMISSION_DENIED); // Use wrong pid and uid assertFalse( @@ -10146,8 +10202,7 @@ public class ConnectivityServiceTest { final NetworkRequest request = new NetworkRequest.Builder().build(); when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder); - mServiceContext.setPermission( - android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); + mServiceContext.setPermission(NETWORK_STACK, PERMISSION_GRANTED); mService.registerConnectivityDiagnosticsCallback( mConnectivityDiagnosticsCallback, request, mContext.getPackageName()); @@ -10166,8 +10221,7 @@ public class ConnectivityServiceTest { final NetworkRequest request = new NetworkRequest.Builder().build(); when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder); - mServiceContext.setPermission( - android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); + mServiceContext.setPermission(NETWORK_STACK, PERMISSION_GRANTED); mService.registerConnectivityDiagnosticsCallback( mConnectivityDiagnosticsCallback, request, mContext.getPackageName()); diff --git a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java index c32c1d2124..3566aef5e4 100644 --- a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java @@ -16,12 +16,17 @@ package com.android.server.net; +import static android.Manifest.permission.READ_NETWORK_USAGE_HISTORY; +import static android.Manifest.permission.UPDATE_DEVICE_STATS; import static android.content.Intent.ACTION_UID_REMOVED; import static android.content.Intent.EXTRA_UID; +import static android.content.pm.PackageManager.PERMISSION_DENIED; +import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.net.ConnectivityManager.TYPE_MOBILE; import static android.net.ConnectivityManager.TYPE_WIFI; import static android.net.NetworkIdentity.OEM_PAID; import static android.net.NetworkIdentity.OEM_PRIVATE; +import static android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK; import static android.net.NetworkStats.DEFAULT_NETWORK_ALL; import static android.net.NetworkStats.DEFAULT_NETWORK_NO; import static android.net.NetworkStats.DEFAULT_NETWORK_YES; @@ -106,6 +111,7 @@ import android.os.SimpleClock; import android.provider.Settings; import android.telephony.TelephonyManager; +import androidx.annotation.Nullable; import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; @@ -200,6 +206,26 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { if (Context.TELEPHONY_SERVICE.equals(name)) return mTelephonyManager; return mBaseContext.getSystemService(name); } + + @Override + public void enforceCallingOrSelfPermission(String permission, @Nullable String message) { + if (checkCallingOrSelfPermission(permission) != PERMISSION_GRANTED) { + throw new SecurityException("Test does not have mocked permission " + permission); + } + } + + @Override + public int checkCallingOrSelfPermission(String permission) { + switch (permission) { + case PERMISSION_MAINLINE_NETWORK_STACK: + case READ_NETWORK_USAGE_HISTORY: + case UPDATE_DEVICE_STATS: + return PERMISSION_GRANTED; + default: + return PERMISSION_DENIED; + } + + } } private final Clock mClock = new SimpleClock(ZoneOffset.UTC) { From fad30e3d847e54c1c0e642be1b81ecec0f510d4b Mon Sep 17 00:00:00 2001 From: Chiachang Wang Date: Wed, 23 Jun 2021 02:08:44 +0000 Subject: [PATCH 2/3] Provide a way to override the avoid bad wifi configuration ConnectivityManager.setAvoidUnvalidated only works if the config_networkAvoidBadWifi configuration is set to 0 and the NETWORK_AVOID_BAD_WIFI setting is unset. There is no easy way for a testing app to temporary set a test value to verify the behavior of the API. Thus, add a mechanism to allow test app to set a period of time to temporary unstrict the resource configuration, i.e. Temporary simulate config_networkAvoidBadWifi configured to 0. Bug: 186061922 Test: atest CtsNetTestCases FrameworksNetTests Original-Change: https://android-review.googlesource.com/1733788 Merged-In: If772078c61a9b12926f104d5dfc9c9071e844732 Change-Id: If772078c61a9b12926f104d5dfc9c9071e844732 --- .../src/android/net/ConnectivityManager.java | 16 ++++ .../src/android/net/IConnectivityManager.aidl | 2 + .../net/util/MultinetworkPolicyTracker.java | 20 ++++- .../android/server/ConnectivityService.java | 31 ++++++++ .../server/ConnectivityServiceTest.java | 75 +++++++++++++------ 5 files changed, 119 insertions(+), 25 deletions(-) diff --git a/framework/src/android/net/ConnectivityManager.java b/framework/src/android/net/ConnectivityManager.java index e49696679b..758c612d46 100644 --- a/framework/src/android/net/ConnectivityManager.java +++ b/framework/src/android/net/ConnectivityManager.java @@ -4721,6 +4721,22 @@ public class ConnectivityManager { } } + /** + * Temporarily allow bad wifi to override {@code config_networkAvoidBadWifi} configuration. + * + * @param timeMs The expired current time. The value should be set within a limited time from + * now. + * + * @hide + */ + public void setTestAllowBadWifiUntil(long timeMs) { + try { + mService.setTestAllowBadWifiUntil(timeMs); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + /** * Requests that the system open the captive portal app on the specified network. * diff --git a/framework/src/android/net/IConnectivityManager.aidl b/framework/src/android/net/IConnectivityManager.aidl index c434bbcb0f..50ec78120f 100644 --- a/framework/src/android/net/IConnectivityManager.aidl +++ b/framework/src/android/net/IConnectivityManager.aidl @@ -226,4 +226,6 @@ interface IConnectivityManager void offerNetwork(int providerId, in NetworkScore score, in NetworkCapabilities caps, in INetworkOfferCallback callback); void unofferNetwork(in INetworkOfferCallback callback); + + void setTestAllowBadWifiUntil(long timeMs); } diff --git a/framework/src/android/net/util/MultinetworkPolicyTracker.java b/framework/src/android/net/util/MultinetworkPolicyTracker.java index 0b42a00369..7e62d288f8 100644 --- a/framework/src/android/net/util/MultinetworkPolicyTracker.java +++ b/framework/src/android/net/util/MultinetworkPolicyTracker.java @@ -75,6 +75,7 @@ public class MultinetworkPolicyTracker { private volatile boolean mAvoidBadWifi = true; private volatile int mMeteredMultipathPreference; private int mActiveSubId = SubscriptionManager.INVALID_SUBSCRIPTION_ID; + private volatile long mTestAllowBadWifiUntilMs = 0; // Mainline module can't use internal HandlerExecutor, so add an identical executor here. private static class HandlerExecutor implements Executor { @@ -162,14 +163,31 @@ public class MultinetworkPolicyTracker { * Whether the device or carrier configuration disables avoiding bad wifi by default. */ public boolean configRestrictsAvoidBadWifi() { + final boolean allowBadWifi = mTestAllowBadWifiUntilMs > 0 + && mTestAllowBadWifiUntilMs > System.currentTimeMillis(); + // If the config returns true, then avoid bad wifi design can be controlled by the + // NETWORK_AVOID_BAD_WIFI setting. + if (allowBadWifi) return true; + // TODO: use R.integer.config_networkAvoidBadWifi directly final int id = mResources.get().getIdentifier("config_networkAvoidBadWifi", "integer", mResources.getResourcesContext().getPackageName()); return (getResourcesForActiveSubId().getInteger(id) == 0); } + /** + * Temporarily allow bad wifi to override {@code config_networkAvoidBadWifi} configuration. + * The value works when the time set is more than {@link System.currentTimeMillis()}. + */ + public void setTestAllowBadWifiUntil(long timeMs) { + Log.d(TAG, "setTestAllowBadWifiUntil: " + mTestAllowBadWifiUntilMs); + mTestAllowBadWifiUntilMs = timeMs; + updateAvoidBadWifi(); + } + + @VisibleForTesting @NonNull - private Resources getResourcesForActiveSubId() { + protected Resources getResourcesForActiveSubId() { return SubscriptionManager.getResourcesForSubId( mResources.getResourcesContext(), mActiveSubId); } diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index f61eee6de3..1f91eca0ed 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -651,6 +651,12 @@ public class ConnectivityService extends IConnectivityManager.Stub */ private static final int EVENT_MOBILE_DATA_PREFERRED_UIDS_CHANGED = 54; + /** + * Event to set temporary allow bad wifi within a limited time to override + * {@code config_networkAvoidBadWifi}. + */ + private static final int EVENT_SET_TEST_ALLOW_BAD_WIFI_UNTIL = 55; + /** * Argument for {@link #EVENT_PROVISIONING_NOTIFICATION} to indicate that the notification * should be shown. @@ -663,6 +669,11 @@ public class ConnectivityService extends IConnectivityManager.Stub */ private static final int PROVISIONING_NOTIFICATION_HIDE = 0; + /** + * The maximum alive time to allow bad wifi configuration for testing. + */ + private static final long MAX_TEST_ALLOW_BAD_WIFI_UNTIL_MS = 5 * 60 * 1000L; + private static String eventName(int what) { return sMagicDecoderRing.get(what, Integer.toString(what)); } @@ -4342,6 +4353,22 @@ public class ConnectivityService extends IConnectivityManager.Stub mHandler.sendMessage(mHandler.obtainMessage(EVENT_SET_AVOID_UNVALIDATED, network)); } + @Override + public void setTestAllowBadWifiUntil(long timeMs) { + enforceSettingsPermission(); + if (!Build.isDebuggable()) { + throw new IllegalStateException("Does not support in non-debuggable build"); + } + + if (timeMs > System.currentTimeMillis() + MAX_TEST_ALLOW_BAD_WIFI_UNTIL_MS) { + throw new IllegalArgumentException("It should not exceed " + + MAX_TEST_ALLOW_BAD_WIFI_UNTIL_MS + "ms from now"); + } + + mHandler.sendMessage( + mHandler.obtainMessage(EVENT_SET_TEST_ALLOW_BAD_WIFI_UNTIL, timeMs)); + } + private void handleSetAcceptUnvalidated(Network network, boolean accept, boolean always) { if (DBG) log("handleSetAcceptUnvalidated network=" + network + " accept=" + accept + " always=" + always); @@ -4884,6 +4911,10 @@ public class ConnectivityService extends IConnectivityManager.Stub case EVENT_MOBILE_DATA_PREFERRED_UIDS_CHANGED: handleMobileDataPreferredUidsChanged(); break; + case EVENT_SET_TEST_ALLOW_BAD_WIFI_UNTIL: + final long timeMs = ((Long) msg.obj).longValue(); + mMultinetworkPolicyTracker.setTestAllowBadWifiUntil(timeMs); + break; } } } diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index dd1e888e01..f6ea964572 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -1496,8 +1496,7 @@ public class ConnectivityServiceTest { return mService.getNetworkAgentInfoForNetwork(mna.getNetwork()).clatd; } - private static class WrappedMultinetworkPolicyTracker extends MultinetworkPolicyTracker { - volatile boolean mConfigRestrictsAvoidBadWifi; + private class WrappedMultinetworkPolicyTracker extends MultinetworkPolicyTracker { volatile int mConfigMeteredMultipathPreference; WrappedMultinetworkPolicyTracker(Context c, Handler h, Runnable r) { @@ -1505,8 +1504,8 @@ public class ConnectivityServiceTest { } @Override - public boolean configRestrictsAvoidBadWifi() { - return mConfigRestrictsAvoidBadWifi; + protected Resources getResourcesForActiveSubId() { + return mResources; } @Override @@ -1723,7 +1722,9 @@ public class ConnectivityServiceTest { .getIdentifier(eq("config_networkSupportedKeepaliveCount"), eq("array"), any()); doReturn(R.array.network_switch_type_name).when(mResources) .getIdentifier(eq("network_switch_type_name"), eq("array"), any()); - + doReturn(R.integer.config_networkAvoidBadWifi).when(mResources) + .getIdentifier(eq("config_networkAvoidBadWifi"), eq("integer"), any()); + doReturn(1).when(mResources).getInteger(R.integer.config_networkAvoidBadWifi); final ConnectivityResources connRes = mock(ConnectivityResources.class); doReturn(mResources).when(connRes).get(); @@ -4763,30 +4764,29 @@ public class ConnectivityServiceTest { } @Test - public void testAvoidBadWifiSetting() throws Exception { + public void testSetAllowBadWifiUntil() throws Exception { + runAsShell(NETWORK_SETTINGS, + () -> mService.setTestAllowBadWifiUntil(System.currentTimeMillis() + 5_000L)); + waitForIdle(); + testAvoidBadWifiConfig_controlledBySettings(); + + runAsShell(NETWORK_SETTINGS, + () -> mService.setTestAllowBadWifiUntil(System.currentTimeMillis() - 5_000L)); + waitForIdle(); + testAvoidBadWifiConfig_ignoreSettings(); + } + + private void testAvoidBadWifiConfig_controlledBySettings() { final ContentResolver cr = mServiceContext.getContentResolver(); final String settingName = ConnectivitySettingsManager.NETWORK_AVOID_BAD_WIFI; - mPolicyTracker.mConfigRestrictsAvoidBadWifi = false; - String[] values = new String[] {null, "0", "1"}; - for (int i = 0; i < values.length; i++) { - Settings.Global.putInt(cr, settingName, 1); - mPolicyTracker.reevaluate(); - waitForIdle(); - String msg = String.format("config=false, setting=%s", values[i]); - assertTrue(mService.avoidBadWifi()); - assertFalse(msg, mPolicyTracker.shouldNotifyWifiUnvalidated()); - } - - mPolicyTracker.mConfigRestrictsAvoidBadWifi = true; - - Settings.Global.putInt(cr, settingName, 0); + Settings.Global.putString(cr, settingName, "0"); mPolicyTracker.reevaluate(); waitForIdle(); assertFalse(mService.avoidBadWifi()); assertFalse(mPolicyTracker.shouldNotifyWifiUnvalidated()); - Settings.Global.putInt(cr, settingName, 1); + Settings.Global.putString(cr, settingName, "1"); mPolicyTracker.reevaluate(); waitForIdle(); assertTrue(mService.avoidBadWifi()); @@ -4799,13 +4799,40 @@ public class ConnectivityServiceTest { assertTrue(mPolicyTracker.shouldNotifyWifiUnvalidated()); } + private void testAvoidBadWifiConfig_ignoreSettings() { + final ContentResolver cr = mServiceContext.getContentResolver(); + final String settingName = ConnectivitySettingsManager.NETWORK_AVOID_BAD_WIFI; + + String[] values = new String[] {null, "0", "1"}; + for (int i = 0; i < values.length; i++) { + Settings.Global.putString(cr, settingName, values[i]); + mPolicyTracker.reevaluate(); + waitForIdle(); + String msg = String.format("config=false, setting=%s", values[i]); + assertTrue(mService.avoidBadWifi()); + assertFalse(msg, mPolicyTracker.shouldNotifyWifiUnvalidated()); + } + } + + @Test + public void testAvoidBadWifiSetting() throws Exception { + final ContentResolver cr = mServiceContext.getContentResolver(); + final String settingName = ConnectivitySettingsManager.NETWORK_AVOID_BAD_WIFI; + + doReturn(1).when(mResources).getInteger(R.integer.config_networkAvoidBadWifi); + testAvoidBadWifiConfig_ignoreSettings(); + + doReturn(0).when(mResources).getInteger(R.integer.config_networkAvoidBadWifi); + testAvoidBadWifiConfig_controlledBySettings(); + } + @Ignore("Refactoring in progress b/178071397") @Test public void testAvoidBadWifi() throws Exception { final ContentResolver cr = mServiceContext.getContentResolver(); // Pretend we're on a carrier that restricts switching away from bad wifi. - mPolicyTracker.mConfigRestrictsAvoidBadWifi = true; + doReturn(0).when(mResources).getInteger(R.integer.config_networkAvoidBadWifi); // File a request for cell to ensure it doesn't go down. final TestNetworkCallback cellNetworkCallback = new TestNetworkCallback(); @@ -4856,13 +4883,13 @@ public class ConnectivityServiceTest { // Simulate switching to a carrier that does not restrict avoiding bad wifi, and expect // that we switch back to cell. - mPolicyTracker.mConfigRestrictsAvoidBadWifi = false; + doReturn(1).when(mResources).getInteger(R.integer.config_networkAvoidBadWifi); mPolicyTracker.reevaluate(); defaultCallback.expectAvailableCallbacksValidated(mCellNetworkAgent); assertEquals(mCm.getActiveNetwork(), cellNetwork); // Switch back to a restrictive carrier. - mPolicyTracker.mConfigRestrictsAvoidBadWifi = true; + doReturn(0).when(mResources).getInteger(R.integer.config_networkAvoidBadWifi); mPolicyTracker.reevaluate(); defaultCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); assertEquals(mCm.getActiveNetwork(), wifiNetwork); From 79f6827d450d715756b3a6f98e7d83cc64797fb4 Mon Sep 17 00:00:00 2001 From: Chiachang Wang Date: Wed, 23 Jun 2021 09:51:28 +0000 Subject: [PATCH 3/3] Add test for CM#setAvoidUnvalidated Bug: 186061922 Test: atest CtsNetTestCases:android.net.cts.ConnectivityManagerTest Test: atest CtsNetTestCasesLatestSdk Original-Change: https://android-review.googlesource.com/1733789 Merged-In: I77dc5556458c2e824f7abd5a7b79e8aeed8dabf3 Change-Id: I77dc5556458c2e824f7abd5a7b79e8aeed8dabf3 --- .../net/cts/ConnectivityManagerTest.java | 100 +++++++++++++++++- 1 file changed, 98 insertions(+), 2 deletions(-) diff --git a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java index c2613c8d96..1b3c2b683d 100644 --- a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java +++ b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java @@ -2399,8 +2399,7 @@ public class ConnectivityManagerTest { } finally { resetValidationConfig(); // Reconnect wifi to reset the wifi status - mCtsNetUtils.ensureWifiDisconnected(null /* wifiNetworkToCheck */); - mCtsNetUtils.ensureWifiConnected(); + reconnectWifi(); } } @@ -2475,6 +2474,88 @@ public class ConnectivityManagerTest { } } + @AppModeFull(reason = "WRITE_DEVICE_CONFIG permission can't be granted to instant apps") + @Test + public void testSetAvoidUnvalidated() throws Exception { + assumeTrue(TestUtils.shouldTestSApis()); + // TODO: Allow in debuggable ROM only. To be replaced by FabricatedOverlay + assumeTrue(Build.isDebuggable()); + final boolean canRunTest = mPackageManager.hasSystemFeature(FEATURE_WIFI) + && mPackageManager.hasSystemFeature(FEATURE_TELEPHONY); + assumeTrue("testSetAvoidUnvalidated cannot execute" + + " unless device supports WiFi and telephony", canRunTest); + + final TestableNetworkCallback wifiCb = new TestableNetworkCallback(); + final TestableNetworkCallback defaultCb = new TestableNetworkCallback(); + final int previousAvoidBadWifi = + ConnectivitySettingsManager.getNetworkAvoidBadWifi(mContext); + + allowBadWifi(); + + final Network cellNetwork = mCtsNetUtils.connectToCell(); + final Network wifiNetwork = prepareValidatedNetwork(); + + mCm.registerDefaultNetworkCallback(defaultCb); + mCm.registerNetworkCallback(makeWifiNetworkRequest(), wifiCb); + + try { + // Verify wifi is the default network. + defaultCb.eventuallyExpect(CallbackEntry.AVAILABLE, NETWORK_CALLBACK_TIMEOUT_MS, + entry -> wifiNetwork.equals(entry.getNetwork())); + wifiCb.eventuallyExpect(CallbackEntry.AVAILABLE, NETWORK_CALLBACK_TIMEOUT_MS, + entry -> wifiNetwork.equals(entry.getNetwork())); + assertTrue(mCm.getNetworkCapabilities(wifiNetwork).hasCapability( + NET_CAPABILITY_VALIDATED)); + + // Configure response code for unvalidated network + configTestServer(Status.INTERNAL_ERROR, Status.INTERNAL_ERROR); + mCm.reportNetworkConnectivity(wifiNetwork, false); + // Default network should stay on unvalidated wifi because avoid bad wifi is disabled. + defaultCb.eventuallyExpect(CallbackEntry.NETWORK_CAPS_UPDATED, + NETWORK_CALLBACK_TIMEOUT_MS, + entry -> !((CallbackEntry.CapabilitiesChanged) entry).getCaps() + .hasCapability(NET_CAPABILITY_VALIDATED)); + wifiCb.eventuallyExpect(CallbackEntry.NETWORK_CAPS_UPDATED, + NETWORK_CALLBACK_TIMEOUT_MS, + entry -> !((CallbackEntry.CapabilitiesChanged) entry).getCaps() + .hasCapability(NET_CAPABILITY_VALIDATED)); + + runAsShell(NETWORK_SETTINGS, () -> { + mCm.setAvoidUnvalidated(wifiNetwork); + }); + // Default network should be updated to validated cellular network. + defaultCb.eventuallyExpect(CallbackEntry.AVAILABLE, NETWORK_CALLBACK_TIMEOUT_MS, + entry -> cellNetwork.equals(entry.getNetwork())); + // No update on wifi callback. + wifiCb.assertNoCallback(); + } finally { + mCm.unregisterNetworkCallback(wifiCb); + mCm.unregisterNetworkCallback(defaultCb); + resetAvoidBadWifi(previousAvoidBadWifi); + resetValidationConfig(); + // Reconnect wifi to reset the wifi status + reconnectWifi(); + } + } + + private void resetAvoidBadWifi(int settingValue) { + setTestAllowBadWifiResource(0 /* timeMs */); + ConnectivitySettingsManager.setNetworkAvoidBadWifi(mContext, settingValue); + } + + private void allowBadWifi() { + setTestAllowBadWifiResource( + System.currentTimeMillis() + WIFI_CONNECT_TIMEOUT_MS /* timeMs */); + ConnectivitySettingsManager.setNetworkAvoidBadWifi(mContext, + ConnectivitySettingsManager.NETWORK_AVOID_BAD_WIFI_IGNORE); + } + + private void setTestAllowBadWifiResource(long timeMs) { + runAsShell(NETWORK_SETTINGS, () -> { + mCm.setTestAllowBadWifiUntil(timeMs); + }); + } + private Network expectNetworkHasCapability(Network network, int expectedNetCap, long timeout) throws Exception { final CompletableFuture future = new CompletableFuture(); @@ -2514,6 +2595,21 @@ public class ConnectivityManagerTest { mHttpServer.start(); } + private Network reconnectWifi() { + mCtsNetUtils.ensureWifiDisconnected(null /* wifiNetworkToCheck */); + return mCtsNetUtils.ensureWifiConnected(); + } + + private Network prepareValidatedNetwork() throws Exception { + prepareHttpServer(); + configTestServer(Status.NO_CONTENT, Status.NO_CONTENT); + // Disconnect wifi first then start wifi network with configuration. + final Network wifiNetwork = reconnectWifi(); + + return expectNetworkHasCapability(wifiNetwork, NET_CAPABILITY_VALIDATED, + WIFI_CONNECT_TIMEOUT_MS); + } + private Network preparePartialConnectivity() throws Exception { prepareHttpServer(); // Configure response code for partial connectivity