From 27b6888364be8e95131252d051b9750119430480 Mon Sep 17 00:00:00 2001 From: Treehugger Robot Date: Mon, 7 Jun 2021 19:42:39 +0000 Subject: [PATCH] Require location permission for ConnDiags WiFi only. This CL updates ConnectivityDiagnostics permission checks in ConnectivityService to only require location permission for Networks that have TRANSPORT_WIFI. This change is consistent with the location restrictions required for the transports themselves. Previously, location permissions were required for all Network types. Bug: 187310575 Test: atest ConnectivityServiceTest ConnectivityDiagnosticsManagerTest Change-Id: I48806533e4e705d2d9be45f3b3d3931d9294b167 Merged-In: I48806533e4e705d2d9be45f3b3d3931d9294b167 (cherry picked from commit 0990af5148693f15d48771aa8063f59c1023eea1) --- .../android/server/ConnectivityService.java | 10 +++- .../server/ConnectivityServiceTest.java | 49 ++++++++++++++++--- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 2ad12a69b0..b4355b9f75 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -78,6 +78,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; @@ -9194,6 +9195,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 @@ -9236,7 +9243,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 d20a08955e..dee312d647 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -9924,10 +9924,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, TEST_LINGER_DELAY_MS, mQosCallbackTracker, @@ -9953,7 +9967,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); @@ -9963,18 +9977,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