Stop using mVpns in getConnectionOwnerUid.

Use data that is already available in ConnectivityService
instead.

The behaviour of the new implementation is slightly different
from Q and R code when the permission check fails.

- The old code would throw a SecurityException if an app that
  was not an active VPN called the method, and would return
  INVALID_UID if the connection belonged to a UID that was not
  subject to the VPN.
- The new code returns INVALID_UID in both cases.

This does not seem like a compatibility problem. The only case in
which the code throws SecurityException is if the app is not a
current VPN app, but the app already knows whether it is or not.
The docs don't mention that the method SecurityException, either.

Bug: 173331190
Test: atest FrameworksNetTests
Test: atest HostsideVpnTests
Change-Id: If3d031e74df33b5c97e12ebf02272faac6769d50
This commit is contained in:
Lorenzo Colitti
2021-02-04 00:18:27 +09:00
parent 31a31193d2
commit a5a903d0b5
2 changed files with 18 additions and 36 deletions

View File

@@ -132,12 +132,14 @@ import android.net.RouteInfo;
import android.net.RouteInfoParcel;
import android.net.SocketKeepalive;
import android.net.TetheringManager;
import android.net.TransportInfo;
import android.net.UidRange;
import android.net.UidRangeParcel;
import android.net.UnderlyingNetworkInfo;
import android.net.Uri;
import android.net.VpnManager;
import android.net.VpnService;
import android.net.VpnTransportInfo;
import android.net.metrics.INetdEventListener;
import android.net.metrics.IpConnectivityLog;
import android.net.metrics.NetworkEvent;
@@ -8477,22 +8479,11 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
}
/**
* Caller either needs to be an active VPN, or hold the NETWORK_STACK permission
* for testing.
*/
private Vpn enforceActiveVpnOrNetworkStackPermission() {
if (checkNetworkStackPermission()) {
return null;
}
synchronized (mVpns) {
Vpn vpn = getVpnIfOwner();
if (vpn != null) {
return vpn;
}
}
throw new SecurityException("App must either be an active VPN or have the NETWORK_STACK "
+ "permission");
private @VpnManager.VpnType int getVpnType(@Nullable NetworkAgentInfo vpn) {
if (vpn == null) return VpnManager.TYPE_VPN_NONE;
final TransportInfo ti = vpn.networkCapabilities.getTransportInfo();
if (!(ti instanceof VpnTransportInfo)) return VpnManager.TYPE_VPN_NONE;
return ((VpnTransportInfo) ti).type;
}
/**
@@ -8502,14 +8493,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
* connection is not found.
*/
public int getConnectionOwnerUid(ConnectionInfo connectionInfo) {
final Vpn vpn = enforceActiveVpnOrNetworkStackPermission();
// Only VpnService based VPNs should be able to get this information.
if (vpn != null && vpn.getActiveVpnType() != VpnManager.TYPE_VPN_SERVICE) {
throw new SecurityException(
"getConnectionOwnerUid() not allowed for non-VpnService VPNs");
}
if (connectionInfo.protocol != IPPROTO_TCP && connectionInfo.protocol != IPPROTO_UDP) {
throw new IllegalArgumentException("Unsupported protocol " + connectionInfo.protocol);
}
@@ -8517,8 +8500,15 @@ public class ConnectivityService extends IConnectivityManager.Stub
final int uid = mDeps.getConnectionOwnerUid(connectionInfo.protocol,
connectionInfo.local, connectionInfo.remote);
/* Filter out Uids not associated with the VPN. */
if (vpn != null && !vpn.appliesToUid(uid)) {
if (uid == INVALID_UID) return uid; // Not found.
// Connection owner UIDs are visible only to the network stack and to the VpnService-based
// VPN, if any, that applies to the UID that owns the connection.
if (checkNetworkStackPermission()) return uid;
final NetworkAgentInfo vpn = getVpnForUid(uid);
if (vpn == null || getVpnType(vpn) != VpnManager.TYPE_VPN_SERVICE
|| vpn.networkCapabilities.getOwnerUid() != Binder.getCallingUid()) {
return INVALID_UID;
}

View File

@@ -8567,11 +8567,7 @@ public class ConnectivityServiceTest {
final int myUid = Process.myUid();
setupConnectionOwnerUidAsVpnApp(myUid, VpnManager.TYPE_VPN_PLATFORM);
try {
mService.getConnectionOwnerUid(getTestConnectionInfo());
fail("Expected SecurityException for non-VpnService app");
} catch (SecurityException expected) {
}
assertEquals(INVALID_UID, mService.getConnectionOwnerUid(getTestConnectionInfo()));
}
@Test
@@ -8579,11 +8575,7 @@ public class ConnectivityServiceTest {
final int myUid = Process.myUid();
setupConnectionOwnerUidAsVpnApp(myUid + 1, VpnManager.TYPE_VPN_SERVICE);
try {
mService.getConnectionOwnerUid(getTestConnectionInfo());
fail("Expected SecurityException for non-VpnService app");
} catch (SecurityException expected) {
}
assertEquals(INVALID_UID, mService.getConnectionOwnerUid(getTestConnectionInfo()));
}
@Test