From ff55aeb9166f89a3ad22bc10287d4e5c0378b606 Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Wed, 16 Jun 2021 11:37:53 +0000 Subject: [PATCH] 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) {