Restrict VPN Diagnostics callbacks to underlying networks.
ConnectivityDiagnosticsCallbacks should only be invoked for the underlying networks declared by active VPNs. This encourages VPN apps to declare their underlying networks. The previous permission model for VPNs allowed active VPNs to receive callbacks on any network. Bug: 148903617 Test: atest FrameworksNetTests Change-Id: Ic08cdd2e2532580fda0fd3034e2bdff27e0ff84b
This commit is contained in:
@@ -7890,10 +7890,13 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
final Network[] underlyingNetworks;
|
||||||
synchronized (mVpns) {
|
synchronized (mVpns) {
|
||||||
if (getVpnIfOwner(callbackUid) != null) {
|
final Vpn vpn = getVpnIfOwner(callbackUid);
|
||||||
return true;
|
underlyingNetworks = (vpn == null) ? null : vpn.getUnderlyingNetworks();
|
||||||
}
|
}
|
||||||
|
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
|
||||||
|
|||||||
@@ -307,6 +307,8 @@ public class ConnectivityServiceTest {
|
|||||||
|
|
||||||
private static final long TIMESTAMP = 1234L;
|
private static final long TIMESTAMP = 1234L;
|
||||||
|
|
||||||
|
private static final int NET_ID = 110;
|
||||||
|
|
||||||
private static final String CLAT_PREFIX = "v4-";
|
private static final String CLAT_PREFIX = "v4-";
|
||||||
private static final String MOBILE_IFNAME = "test_rmnet_data0";
|
private static final String MOBILE_IFNAME = "test_rmnet_data0";
|
||||||
private static final String WIFI_IFNAME = "test_wlan0";
|
private static final String WIFI_IFNAME = "test_wlan0";
|
||||||
@@ -1016,6 +1018,7 @@ public class ConnectivityServiceTest {
|
|||||||
private int mVpnType = VpnManager.TYPE_VPN_SERVICE;
|
private int mVpnType = VpnManager.TYPE_VPN_SERVICE;
|
||||||
|
|
||||||
private VpnInfo mVpnInfo;
|
private VpnInfo mVpnInfo;
|
||||||
|
private Network[] mUnderlyingNetworks;
|
||||||
|
|
||||||
public MockVpn(int userId) {
|
public MockVpn(int userId) {
|
||||||
super(startHandlerThreadAndReturnLooper(), mServiceContext, mNetworkManagementService,
|
super(startHandlerThreadAndReturnLooper(), mServiceContext, mNetworkManagementService,
|
||||||
@@ -1105,9 +1108,21 @@ public class ConnectivityServiceTest {
|
|||||||
return super.getVpnInfo();
|
return super.getVpnInfo();
|
||||||
}
|
}
|
||||||
|
|
||||||
private void setVpnInfo(VpnInfo vpnInfo) {
|
private synchronized void setVpnInfo(VpnInfo vpnInfo) {
|
||||||
mVpnInfo = vpnInfo;
|
mVpnInfo = vpnInfo;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public synchronized Network[] getUnderlyingNetworks() {
|
||||||
|
if (mUnderlyingNetworks != null) return mUnderlyingNetworks;
|
||||||
|
|
||||||
|
return super.getUnderlyingNetworks();
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Don't override behavior for {@link Vpn#setUnderlyingNetworks}. */
|
||||||
|
private synchronized void overrideUnderlyingNetworks(Network[] underlyingNetworks) {
|
||||||
|
mUnderlyingNetworks = underlyingNetworks;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private void mockVpn(int uid) {
|
private void mockVpn(int uid) {
|
||||||
@@ -6774,9 +6789,10 @@ public class ConnectivityServiceTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testCheckConnectivityDiagnosticsPermissionsActiveVpn() throws Exception {
|
public void testCheckConnectivityDiagnosticsPermissionsActiveVpn() throws Exception {
|
||||||
|
final Network network = new Network(NET_ID);
|
||||||
final NetworkAgentInfo naiWithoutUid =
|
final NetworkAgentInfo naiWithoutUid =
|
||||||
new NetworkAgentInfo(
|
new NetworkAgentInfo(
|
||||||
null, null, null, null, null, new NetworkCapabilities(), 0,
|
null, null, network, null, null, new NetworkCapabilities(), 0,
|
||||||
mServiceContext, null, null, mService, null, null, null, 0);
|
mServiceContext, null, null, mService, null, null, null, 0);
|
||||||
|
|
||||||
setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
|
setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
|
||||||
@@ -6789,11 +6805,19 @@ public class ConnectivityServiceTest {
|
|||||||
info.ownerUid = Process.myUid();
|
info.ownerUid = Process.myUid();
|
||||||
info.vpnIface = "interface";
|
info.vpnIface = "interface";
|
||||||
mMockVpn.setVpnInfo(info);
|
mMockVpn.setVpnInfo(info);
|
||||||
|
mMockVpn.overrideUnderlyingNetworks(new Network[] {network});
|
||||||
assertTrue(
|
assertTrue(
|
||||||
"Active VPN permission not applied",
|
"Active VPN permission not applied",
|
||||||
mService.checkConnectivityDiagnosticsPermissions(
|
mService.checkConnectivityDiagnosticsPermissions(
|
||||||
Process.myPid(), Process.myUid(), naiWithoutUid,
|
Process.myPid(), Process.myUid(), naiWithoutUid,
|
||||||
mContext.getOpPackageName()));
|
mContext.getOpPackageName()));
|
||||||
|
|
||||||
|
mMockVpn.overrideUnderlyingNetworks(null);
|
||||||
|
assertFalse(
|
||||||
|
"VPN shouldn't receive callback on non-underlying network",
|
||||||
|
mService.checkConnectivityDiagnosticsPermissions(
|
||||||
|
Process.myPid(), Process.myUid(), naiWithoutUid,
|
||||||
|
mContext.getOpPackageName()));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
Reference in New Issue
Block a user