Stop accessing VPNs in checkConnectivityDiagnosticsPermissions.

Currently, checkConnectivityDiagnosticsPermissions takes the VPN
lock to examine the VPN's underlying networks. Use the underlying
network data that is available in ConnectivityService instead.

Bug: 173331190
Test: passes existing tests in ConnectivityServiceTest
Change-Id: Ia1c366c5e9974d4d2c4b38030e66c007d62020ff
This commit is contained in:
Lorenzo Colitti
2020-11-16 23:41:21 +09:00
parent fee5e4e34c
commit 083b00b5fb
2 changed files with 10 additions and 16 deletions

View File

@@ -8276,13 +8276,12 @@ public class ConnectivityService extends IConnectivityManager.Stub
return false; return false;
} }
final Network[] underlyingNetworks; for (NetworkAgentInfo virtual : mNetworkAgentInfos.values()) {
synchronized (mVpns) { if (virtual.supportsUnderlyingNetworks()
final Vpn vpn = getVpnIfOwner(callbackUid); && virtual.networkCapabilities.getOwnerUid() == callbackUid
underlyingNetworks = (vpn == null) ? null : vpn.getUnderlyingNetworks(); && ArrayUtils.contains(virtual.declaredUnderlyingNetworks, nai.network)) {
} return true;
if (underlyingNetworks != null) { }
if (Arrays.asList(underlyingNetworks).contains(nai.network)) return true;
} }
// Administrator UIDs also contains the Owner UID // Administrator UIDs also contains the Owner UID

View File

@@ -7438,20 +7438,14 @@ public class ConnectivityServiceTest {
setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
Manifest.permission.ACCESS_FINE_LOCATION); Manifest.permission.ACCESS_FINE_LOCATION);
// setUp() calls mockVpn() which adds a VPN with the Test Runner's uid. Configure it to be
// active
final VpnInfo info = new VpnInfo();
info.ownerUid = Process.myUid();
info.vpnIface = VPN_IFNAME;
mMockVpn.setVpnInfo(info);
mMockVpn.establishForMyUid(); mMockVpn.establishForMyUid();
waitForIdle();
// Wait for networks to connect and broadcasts to be sent before removing permissions.
waitForIdle();
mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED);
assertTrue(mService.setUnderlyingNetworksForVpn(new Network[] {network})); assertTrue(mService.setUnderlyingNetworksForVpn(new Network[] {network}));
waitForIdle();
assertTrue( assertTrue(
"Active VPN permission not applied", "Active VPN permission not applied",
mService.checkConnectivityDiagnosticsPermissions( mService.checkConnectivityDiagnosticsPermissions(
@@ -7459,6 +7453,7 @@ public class ConnectivityServiceTest {
mContext.getOpPackageName())); mContext.getOpPackageName()));
assertTrue(mService.setUnderlyingNetworksForVpn(null)); assertTrue(mService.setUnderlyingNetworksForVpn(null));
waitForIdle();
assertFalse( assertFalse(
"VPN shouldn't receive callback on non-underlying network", "VPN shouldn't receive callback on non-underlying network",
mService.checkConnectivityDiagnosticsPermissions( mService.checkConnectivityDiagnosticsPermissions(