diff --git a/framework/src/android/net/ConnectivityDiagnosticsManager.java b/framework/src/android/net/ConnectivityDiagnosticsManager.java index 3598ebc70118e93b71df18b940d562e0e7460622..dcc8a5eacd1386e03688035e54f68465ef25fe0b 100644 --- a/framework/src/android/net/ConnectivityDiagnosticsManager.java +++ b/framework/src/android/net/ConnectivityDiagnosticsManager.java @@ -713,7 +713,9 @@ public class ConnectivityDiagnosticsManager { * <p>Callbacks registered by apps not meeting the above criteria will not be invoked. * * <p>If a registering app loses its relevant permissions, any callbacks it registered will - * silently stop receiving callbacks. + * silently stop receiving callbacks. Note that registering apps must also have location + * permissions to receive callbacks as some Networks may be location-bound (such as WiFi + * networks). * * <p>Each register() call <b>MUST</b> use a ConnectivityDiagnosticsCallback instance that is * not currently registered. If a ConnectivityDiagnosticsCallback instance is registered with diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 29a485680667a787f62f683a64e09d05b1e74404..5c47f27ef58f17c4c8a6af9e140f73f59719516d 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -9162,36 +9162,49 @@ public class ConnectivityService extends IConnectivityManager.Stub return results; } - @VisibleForTesting - boolean checkConnectivityDiagnosticsPermissions( - int callbackPid, int callbackUid, NetworkAgentInfo nai, String callbackPackageName) { - if (checkNetworkStackPermission(callbackPid, callbackUid)) { - return true; - } - + 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 // call in a try-catch. try { if (!mLocationPermissionChecker.checkLocationPermission( - callbackPackageName, null /* featureId */, callbackUid, null /* message */)) { + packageName, null /* featureId */, uid, null /* message */)) { return false; } } catch (SecurityException e) { return false; } + return true; + } + + private boolean ownsVpnRunningOverNetwork(int uid, Network network) { for (NetworkAgentInfo virtual : mNetworkAgentInfos) { if (virtual.supportsUnderlyingNetworks() - && virtual.networkCapabilities.getOwnerUid() == callbackUid - && CollectionUtils.contains(virtual.declaredUnderlyingNetworks, nai.network)) { + && virtual.networkCapabilities.getOwnerUid() == uid + && CollectionUtils.contains(virtual.declaredUnderlyingNetworks, network)) { return true; } } + return false; + } + + @VisibleForTesting + boolean checkConnectivityDiagnosticsPermissions( + int callbackPid, int callbackUid, NetworkAgentInfo nai, String callbackPackageName) { + if (checkNetworkStackPermission(callbackPid, callbackUid)) { + return true; + } + // Administrator UIDs also contains the Owner UID final int[] administratorUids = nai.networkCapabilities.getAdministratorUids(); - return CollectionUtils.contains(administratorUids, callbackUid); + if (!CollectionUtils.contains(administratorUids, callbackUid) + && !ownsVpnRunningOverNetwork(callbackUid, nai.network)) { + return false; + } + + return 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 29a411e2c9358a8182cb93297e7799ab48c8c5dd..466138550aae76b44d860043eac86dc8c1ad237f 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -9940,28 +9940,32 @@ public class ConnectivityServiceTest { @Test public void testCheckConnectivityDiagnosticsPermissionsWrongUidPackageName() throws Exception { - final NetworkAgentInfo naiWithoutUid = fakeMobileNai(new NetworkCapabilities()); + final int wrongUid = Process.myUid() + 1; + + final NetworkCapabilities nc = new NetworkCapabilities(); + nc.setAdministratorUids(new int[] {wrongUid}); + final NetworkAgentInfo naiWithUid = fakeMobileNai(nc); mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); assertFalse( "Mismatched uid/package name should not pass the location permission check", mService.checkConnectivityDiagnosticsPermissions( - Process.myPid() + 1, Process.myUid() + 1, naiWithoutUid, - mContext.getOpPackageName())); + Process.myPid() + 1, wrongUid, naiWithUid, mContext.getOpPackageName())); } @Test public void testCheckConnectivityDiagnosticsPermissionsNoLocationPermission() throws Exception { - final NetworkAgentInfo naiWithoutUid = fakeMobileNai(new NetworkCapabilities()); + 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); assertFalse( "ACCESS_FINE_LOCATION permission necessary for Connectivity Diagnostics", mService.checkConnectivityDiagnosticsPermissions( - Process.myPid(), Process.myUid(), naiWithoutUid, - mContext.getOpPackageName())); + Process.myPid(), Process.myUid(), naiWithUid, mContext.getOpPackageName())); } @Test