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
This commit is contained in:
Cody Kesting
2021-05-11 14:22:40 -07:00
parent fc592e8bc0
commit 7474f67454
2 changed files with 50 additions and 9 deletions

View File

@@ -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_CELLULAR;
import static android.net.NetworkCapabilities.TRANSPORT_TEST; import static android.net.NetworkCapabilities.TRANSPORT_TEST;
import static android.net.NetworkCapabilities.TRANSPORT_VPN; 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.NetworkRequest.Type.LISTEN_FOR_BEST;
import static android.net.shared.NetworkMonitorUtils.isPrivateDnsValidationRequired; import static android.net.shared.NetworkMonitorUtils.isPrivateDnsValidationRequired;
import static android.os.Process.INVALID_UID; import static android.os.Process.INVALID_UID;
@@ -9175,6 +9176,12 @@ public class ConnectivityService extends IConnectivityManager.Stub
return results; 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) { private boolean hasLocationPermission(String packageName, int uid) {
// LocationPermissionChecker#checkLocationPermission can throw SecurityException if the 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 // and package name don't match. Throwing on the CS thread is not acceptable, so wrap the
@@ -9217,7 +9224,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
return false; return false;
} }
return hasLocationPermission(callbackPackageName, callbackUid); return !isLocationPermissionRequiredForConnectivityDiagnostics(nai)
|| hasLocationPermission(callbackPackageName, callbackUid);
} }
@Override @Override

View File

@@ -9719,10 +9719,24 @@ public class ConnectivityServiceTest {
} }
public NetworkAgentInfo fakeMobileNai(NetworkCapabilities nc) { 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, final NetworkInfo info = new NetworkInfo(TYPE_MOBILE, TelephonyManager.NETWORK_TYPE_LTE,
ConnectivityManager.getNetworkTypeName(TYPE_MOBILE), ConnectivityManager.getNetworkTypeName(TYPE_MOBILE),
TelephonyManager.getNetworkTypeName(TelephonyManager.NETWORK_TYPE_LTE)); 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(), nc, new NetworkScore.Builder().setLegacyInt(0).build(),
mServiceContext, null, new NetworkAgentConfig(), mService, null, null, 0, mServiceContext, null, new NetworkAgentConfig(), mService, null, null, 0,
INVALID_UID, mQosCallbackTracker, new ConnectivityService.Dependencies()); INVALID_UID, mQosCallbackTracker, new ConnectivityService.Dependencies());
@@ -9747,7 +9761,7 @@ public class ConnectivityServiceTest {
final NetworkCapabilities nc = new NetworkCapabilities(); final NetworkCapabilities nc = new NetworkCapabilities();
nc.setAdministratorUids(new int[] {wrongUid}); nc.setAdministratorUids(new int[] {wrongUid});
final NetworkAgentInfo naiWithUid = fakeMobileNai(nc); final NetworkAgentInfo naiWithUid = fakeWifiNai(nc);
mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED);
@@ -9757,18 +9771,37 @@ public class ConnectivityServiceTest {
Process.myPid() + 1, wrongUid, naiWithUid, mContext.getOpPackageName())); 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 @Test
public void testCheckConnectivityDiagnosticsPermissionsNoLocationPermission() throws Exception { public void testCheckConnectivityDiagnosticsPermissionsCellularNoLocationPermission()
throws Exception {
final NetworkCapabilities nc = new NetworkCapabilities(); final NetworkCapabilities nc = new NetworkCapabilities();
nc.setAdministratorUids(new int[] {Process.myUid()}); nc.setAdministratorUids(new int[] {Process.myUid()});
final NetworkAgentInfo naiWithUid = fakeMobileNai(nc); final NetworkAgentInfo naiWithUid = fakeMobileNai(nc);
mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); verifyConnectivityDiagnosticsPermissionsWithNetworkAgentInfo(naiWithUid,
true /* expectPermission */);
}
assertFalse( @Test
"ACCESS_FINE_LOCATION permission necessary for Connectivity Diagnostics", public void testCheckConnectivityDiagnosticsPermissionsWifiNoLocationPermission()
mService.checkConnectivityDiagnosticsPermissions( throws Exception {
Process.myPid(), Process.myUid(), naiWithUid, mContext.getOpPackageName())); final NetworkCapabilities nc = new NetworkCapabilities();
nc.setAdministratorUids(new int[] {Process.myUid()});
final NetworkAgentInfo naiWithUid = fakeWifiNai(nc);
verifyConnectivityDiagnosticsPermissionsWithNetworkAgentInfo(naiWithUid,
false /* expectPermission */);
} }
@Test @Test