diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index a16db11e73..f57761fc48 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -77,6 +77,7 @@ import static android.net.NetworkCapabilities.REDACT_FOR_NETWORK_SETTINGS; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.NetworkCapabilities.TRANSPORT_TEST; import static android.net.NetworkCapabilities.TRANSPORT_VPN; +import static android.net.NetworkCapabilities.TRANSPORT_WIFI; import static android.net.NetworkRequest.Type.LISTEN_FOR_BEST; import static android.net.shared.NetworkMonitorUtils.isPrivateDnsValidationRequired; import static android.os.Process.INVALID_UID; @@ -9186,6 +9187,12 @@ public class ConnectivityService extends IConnectivityManager.Stub return results; } + private boolean isLocationPermissionRequiredForConnectivityDiagnostics( + @NonNull NetworkAgentInfo nai) { + // TODO(b/188483916): replace with a transport-agnostic location-aware check + return nai.networkCapabilities.hasTransport(TRANSPORT_WIFI); + } + private boolean hasLocationPermission(String packageName, int uid) { // LocationPermissionChecker#checkLocationPermission can throw SecurityException if the uid // and package name don't match. Throwing on the CS thread is not acceptable, so wrap the @@ -9228,7 +9235,8 @@ public class ConnectivityService extends IConnectivityManager.Stub return false; } - return hasLocationPermission(callbackPackageName, callbackUid); + return !isLocationPermissionRequiredForConnectivityDiagnostics(nai) + || hasLocationPermission(callbackPackageName, callbackUid); } @Override diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index a42adbd1bb..7d6f95f825 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -9827,10 +9827,24 @@ public class ConnectivityServiceTest { } public NetworkAgentInfo fakeMobileNai(NetworkCapabilities nc) { + final NetworkCapabilities cellNc = new NetworkCapabilities.Builder(nc) + .addTransportType(TRANSPORT_CELLULAR).build(); final NetworkInfo info = new NetworkInfo(TYPE_MOBILE, TelephonyManager.NETWORK_TYPE_LTE, ConnectivityManager.getNetworkTypeName(TYPE_MOBILE), TelephonyManager.getNetworkTypeName(TelephonyManager.NETWORK_TYPE_LTE)); - return new NetworkAgentInfo(null, new Network(NET_ID), info, new LinkProperties(), + return fakeNai(cellNc, info); + } + + private NetworkAgentInfo fakeWifiNai(NetworkCapabilities nc) { + final NetworkCapabilities wifiNc = new NetworkCapabilities.Builder(nc) + .addTransportType(TRANSPORT_WIFI).build(); + final NetworkInfo info = new NetworkInfo(TYPE_WIFI, 0 /* subtype */, + ConnectivityManager.getNetworkTypeName(TYPE_WIFI), "" /* subtypeName */); + return fakeNai(wifiNc, info); + } + + private NetworkAgentInfo fakeNai(NetworkCapabilities nc, NetworkInfo networkInfo) { + return new NetworkAgentInfo(null, new Network(NET_ID), networkInfo, new LinkProperties(), nc, new NetworkScore.Builder().setLegacyInt(0).build(), mServiceContext, null, new NetworkAgentConfig(), mService, null, null, 0, INVALID_UID, mQosCallbackTracker, new ConnectivityService.Dependencies()); @@ -9855,7 +9869,7 @@ public class ConnectivityServiceTest { final NetworkCapabilities nc = new NetworkCapabilities(); nc.setAdministratorUids(new int[] {wrongUid}); - final NetworkAgentInfo naiWithUid = fakeMobileNai(nc); + final NetworkAgentInfo naiWithUid = fakeWifiNai(nc); mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); @@ -9865,18 +9879,37 @@ public class ConnectivityServiceTest { Process.myPid() + 1, wrongUid, naiWithUid, mContext.getOpPackageName())); } + private void verifyConnectivityDiagnosticsPermissionsWithNetworkAgentInfo( + NetworkAgentInfo info, boolean expectPermission) { + mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + + assertEquals( + "Unexpected ConnDiags permission", + expectPermission, + mService.checkConnectivityDiagnosticsPermissions( + Process.myPid(), Process.myUid(), info, mContext.getOpPackageName())); + } + @Test - public void testCheckConnectivityDiagnosticsPermissionsNoLocationPermission() throws Exception { + public void testCheckConnectivityDiagnosticsPermissionsCellularNoLocationPermission() + throws Exception { final NetworkCapabilities nc = new NetworkCapabilities(); nc.setAdministratorUids(new int[] {Process.myUid()}); final NetworkAgentInfo naiWithUid = fakeMobileNai(nc); - mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + verifyConnectivityDiagnosticsPermissionsWithNetworkAgentInfo(naiWithUid, + true /* expectPermission */); + } - assertFalse( - "ACCESS_FINE_LOCATION permission necessary for Connectivity Diagnostics", - mService.checkConnectivityDiagnosticsPermissions( - Process.myPid(), Process.myUid(), naiWithUid, mContext.getOpPackageName())); + @Test + public void testCheckConnectivityDiagnosticsPermissionsWifiNoLocationPermission() + throws Exception { + final NetworkCapabilities nc = new NetworkCapabilities(); + nc.setAdministratorUids(new int[] {Process.myUid()}); + final NetworkAgentInfo naiWithUid = fakeWifiNai(nc); + + verifyConnectivityDiagnosticsPermissionsWithNetworkAgentInfo(naiWithUid, + false /* expectPermission */); } @Test