diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 0a0b5cb0de..15b666a6eb 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -1247,13 +1247,6 @@ public class ConnectivityService extends IConnectivityManager.Stub public boolean getCellular464XlatEnabled() { return NetworkProperties.isCellular464XlatEnabled().orElse(true); } - - /** - * Create a {@link LocationPermissionChecker}. - */ - public LocationPermissionChecker makeLocationPermissionChecker(Context context) { - return new LocationPermissionChecker(context); - } } public ConnectivityService(Context context) { @@ -1321,7 +1314,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 = mDeps.makeLocationPermissionChecker(mContext); + mLocationPermissionChecker = new LocationPermissionChecker(mContext); // To ensure uid state is synchronized with Network Policy, register for // NetworkPolicyManagerService events must happen prior to NetworkPolicyManagerService diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 78547ffbfa..a9612def0c 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -18,15 +18,10 @@ 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; @@ -135,7 +130,6 @@ 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; @@ -261,7 +255,6 @@ 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; @@ -308,7 +301,6 @@ 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; @@ -471,7 +463,6 @@ public class ConnectivityServiceTest { @Mock DeviceIdleInternal mDeviceIdleInternal; @Mock INetworkManagementService mNetworkManagementService; @Mock NetworkStatsManager mStatsManager; - @Mock BatteryStatsManager mBatteryStatsManager; @Mock IDnsResolver mMockDnsResolver; @Mock INetd mMockNetd; @Mock NetworkStackClientBase mNetworkStack; @@ -582,7 +573,6 @@ 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); } @@ -663,13 +653,6 @@ 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) { - return null; - } } private void waitForIdle() { @@ -1219,22 +1202,7 @@ public class ConnectivityServiceTest { return mDeviceIdleInternal; } }, - 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. - case Settings.Secure.ALWAYS_ON_VPN_APP: - return null; - default: - return super.settingsSecureGetStringForUser(key, userId); - } - } - }, new Ikev2SessionCreator()); + mNetworkManagementService, mMockNetd, userId, mVpnProfileStore); } public void setUids(Set uids) { @@ -1612,11 +1580,6 @@ 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(); @@ -1676,13 +1639,6 @@ 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); @@ -1854,16 +1810,6 @@ public class ConnectivityServiceTest { assertEmpty(mCm.getAllNetworkStateSnapshots()); } - private static PendingIntent wrapPendingIntent(final PendingIntent intent) { - final PendingIntent ret = spy(intent); - // intentFilterEquals requires GET_INTENT_SENDER_INTENT permission - doAnswer(inv -> { - final PendingIntent other = inv.getArgument(0); - return runAsShell(GET_INTENT_SENDER_INTENT, () -> intent.intentFilterEquals(other)); - }).when(ret).intentFilterEquals(any()); - return ret; - } - /** * Class to simplify expecting broadcasts using BroadcastInterceptingContext. * Ensures that the receiver is unregistered after the expected broadcast is received. This @@ -3312,8 +3258,8 @@ public class ConnectivityServiceTest { @Test public void testNoMutableNetworkRequests() throws Exception { - final PendingIntent pendingIntent = wrapPendingIntent(PendingIntent.getBroadcast( - mContext, 0 /* requestCode */, new Intent("a"), FLAG_IMMUTABLE)); + final PendingIntent pendingIntent = PendingIntent.getBroadcast( + mContext, 0 /* requestCode */, new Intent("a"), FLAG_IMMUTABLE); NetworkRequest request1 = new NetworkRequest.Builder() .addCapability(NET_CAPABILITY_VALIDATED) .build(); @@ -4157,16 +4103,16 @@ public class ConnectivityServiceTest { mCm.registerNetworkCallback(r, new NetworkCallback())); assertThrows(SecurityException.class, () -> - mCm.registerNetworkCallback(r, wrapPendingIntent(PendingIntent.getService( - mServiceContext, 0 /* requestCode */, new Intent(), FLAG_IMMUTABLE)))); + mCm.registerNetworkCallback(r, PendingIntent.getService( + mServiceContext, 0 /* requestCode */, new Intent(), FLAG_IMMUTABLE))); // Requesting a Network with signal strength should get IllegalArgumentException. assertThrows(IllegalArgumentException.class, () -> mCm.requestNetwork(r, new NetworkCallback())); assertThrows(IllegalArgumentException.class, () -> - mCm.requestNetwork(r, wrapPendingIntent(PendingIntent.getService( - mServiceContext, 0 /* requestCode */, new Intent(), FLAG_IMMUTABLE)))); + mCm.requestNetwork(r, PendingIntent.getService( + mServiceContext, 0 /* requestCode */, new Intent(), FLAG_IMMUTABLE))); } @Test @@ -5722,14 +5668,14 @@ public class ConnectivityServiceTest { } j = 0; while (j++ < INTENTS / 2) { - final PendingIntent pi = wrapPendingIntent(PendingIntent.getBroadcast(mContext, - 0 /* requestCode */, new Intent("a" + j), FLAG_IMMUTABLE)); + final PendingIntent pi = PendingIntent.getBroadcast(mContext, 0 /* requestCode */, + new Intent("a" + j), FLAG_IMMUTABLE); mCm.requestNetwork(networkRequest, pi); registered.add(pi); } while (j++ < INTENTS) { - final PendingIntent pi = wrapPendingIntent(PendingIntent.getBroadcast(mContext, - 0 /* requestCode */, new Intent("b" + j), FLAG_IMMUTABLE)); + final PendingIntent pi = PendingIntent.getBroadcast(mContext, 0 /* requestCode */, + new Intent("b" + j), FLAG_IMMUTABLE); mCm.registerNetworkCallback(networkRequest, pi); registered.add(pi); } @@ -5743,13 +5689,13 @@ public class ConnectivityServiceTest { ); assertThrows(TooManyRequestsException.class, () -> mCm.requestNetwork(networkRequest, - wrapPendingIntent(PendingIntent.getBroadcast(mContext, 0 /* requestCode */, - new Intent("c"), FLAG_IMMUTABLE))) + PendingIntent.getBroadcast(mContext, 0 /* requestCode */, + new Intent("c"), FLAG_IMMUTABLE)) ); assertThrows(TooManyRequestsException.class, () -> mCm.registerNetworkCallback(networkRequest, - wrapPendingIntent(PendingIntent.getBroadcast(mContext, 0 /* requestCode */, - new Intent("d"), FLAG_IMMUTABLE))) + PendingIntent.getBroadcast(mContext, 0 /* requestCode */, + new Intent("d"), FLAG_IMMUTABLE)) ); // The system gets another SYSTEM_ONLY_MAX_REQUESTS slots. @@ -5829,16 +5775,16 @@ public class ConnectivityServiceTest { waitForIdle(); for (int i = 0; i < MAX_REQUESTS; i++) { - final PendingIntent pendingIntent = wrapPendingIntent(PendingIntent.getBroadcast( - mContext, 0 /* requestCode */, new Intent("e" + i), FLAG_IMMUTABLE)); + final PendingIntent pendingIntent = PendingIntent.getBroadcast( + mContext, 0 /* requestCode */, new Intent("e" + i), FLAG_IMMUTABLE); mCm.requestNetwork(networkRequest, pendingIntent); mCm.unregisterNetworkCallback(pendingIntent); } waitForIdle(); for (int i = 0; i < MAX_REQUESTS; i++) { - final PendingIntent pendingIntent = wrapPendingIntent(PendingIntent.getBroadcast( - mContext, 0 /* requestCode */, new Intent("f" + i), FLAG_IMMUTABLE)); + final PendingIntent pendingIntent = PendingIntent.getBroadcast( + mContext, 0 /* requestCode */, new Intent("f" + i), FLAG_IMMUTABLE); mCm.registerNetworkCallback(networkRequest, pendingIntent); mCm.unregisterNetworkCallback(pendingIntent); } @@ -9281,7 +9227,8 @@ public class ConnectivityServiceTest { mServiceContext.setPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, PERMISSION_DENIED); mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_DENIED); - mServiceContext.setPermission(NETWORK_STACK, PERMISSION_DENIED); + mServiceContext.setPermission(Manifest.permission.NETWORK_STACK, + PERMISSION_DENIED); mServiceContext.setPermission(Manifest.permission.NETWORK_SETUP_WIZARD, PERMISSION_DENIED); } @@ -9722,7 +9669,7 @@ public class ConnectivityServiceTest { setupConnectionOwnerUid(vpnOwnerUid, vpnType); // Test as VPN app - mServiceContext.setPermission(NETWORK_STACK, PERMISSION_DENIED); + mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); mServiceContext.setPermission( NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, PERMISSION_DENIED); } @@ -9762,7 +9709,8 @@ public class ConnectivityServiceTest { public void testGetConnectionOwnerUidVpnServiceNetworkStackDoesNotThrow() throws Exception { final int myUid = Process.myUid(); setupConnectionOwnerUid(myUid, VpnManager.TYPE_VPN_SERVICE); - mServiceContext.setPermission(NETWORK_STACK, PERMISSION_GRANTED); + mServiceContext.setPermission( + android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); assertEquals(42, mService.getConnectionOwnerUid(getTestConnectionInfo())); } @@ -9930,7 +9878,8 @@ public class ConnectivityServiceTest { public void testCheckConnectivityDiagnosticsPermissionsNetworkStack() throws Exception { final NetworkAgentInfo naiWithoutUid = fakeMobileNai(new NetworkCapabilities()); - mServiceContext.setPermission(NETWORK_STACK, PERMISSION_GRANTED); + mServiceContext.setPermission( + android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); assertTrue( "NetworkStack permission not applied", mService.checkConnectivityDiagnosticsPermissions( @@ -9946,7 +9895,7 @@ public class ConnectivityServiceTest { nc.setAdministratorUids(new int[] {wrongUid}); final NetworkAgentInfo naiWithUid = fakeWifiNai(nc); - mServiceContext.setPermission(NETWORK_STACK, PERMISSION_DENIED); + mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); assertFalse( "Mismatched uid/package name should not pass the location permission check", @@ -9956,7 +9905,7 @@ public class ConnectivityServiceTest { private void verifyConnectivityDiagnosticsPermissionsWithNetworkAgentInfo( NetworkAgentInfo info, boolean expectPermission) { - mServiceContext.setPermission(NETWORK_STACK, PERMISSION_DENIED); + mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); assertEquals( "Unexpected ConnDiags permission", @@ -10024,7 +9973,7 @@ public class ConnectivityServiceTest { setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION); - mServiceContext.setPermission(NETWORK_STACK, PERMISSION_DENIED); + mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); assertTrue( "NetworkCapabilities administrator uid permission not applied", @@ -10041,7 +9990,7 @@ public class ConnectivityServiceTest { setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION); - mServiceContext.setPermission(NETWORK_STACK, PERMISSION_DENIED); + mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); // Use wrong pid and uid assertFalse( @@ -10067,7 +10016,8 @@ public class ConnectivityServiceTest { final NetworkRequest request = new NetworkRequest.Builder().build(); when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder); - mServiceContext.setPermission(NETWORK_STACK, PERMISSION_GRANTED); + mServiceContext.setPermission( + android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); mService.registerConnectivityDiagnosticsCallback( mConnectivityDiagnosticsCallback, request, mContext.getPackageName()); @@ -10086,7 +10036,8 @@ public class ConnectivityServiceTest { final NetworkRequest request = new NetworkRequest.Builder().build(); when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder); - mServiceContext.setPermission(NETWORK_STACK, PERMISSION_GRANTED); + mServiceContext.setPermission( + android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); mService.registerConnectivityDiagnosticsCallback( mConnectivityDiagnosticsCallback, request, mContext.getPackageName()); @@ -12817,8 +12768,8 @@ public class ConnectivityServiceTest { @Test public void testNetworkRequestWithSubIdsWithNetworkFactoryPermission() throws Exception { mServiceContext.setPermission(NETWORK_FACTORY, PERMISSION_GRANTED); - final PendingIntent pendingIntent = wrapPendingIntent(PendingIntent.getBroadcast( - mContext, 0 /* requestCode */, new Intent("a"), FLAG_IMMUTABLE)); + final PendingIntent pendingIntent = PendingIntent.getBroadcast( + mContext, 0 /* requestCode */, new Intent("a"), FLAG_IMMUTABLE); final NetworkCallback networkCallback1 = new NetworkCallback(); final NetworkCallback networkCallback2 = new NetworkCallback(); @@ -12834,8 +12785,8 @@ public class ConnectivityServiceTest { @Test public void testNetworkRequestWithSubIdsWithoutNetworkFactoryPermission() throws Exception { mServiceContext.setPermission(NETWORK_FACTORY, PERMISSION_DENIED); - final PendingIntent pendingIntent = wrapPendingIntent(PendingIntent.getBroadcast( - mContext, 0 /* requestCode */, new Intent("a"), FLAG_IMMUTABLE)); + final PendingIntent pendingIntent = PendingIntent.getBroadcast( + mContext, 0 /* requestCode */, new Intent("a"), FLAG_IMMUTABLE); final Class expected = SecurityException.class; assertThrows(