Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9cc08edf authored by Lorenzo Colitti's avatar Lorenzo Colitti
Browse files

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
parent 76e8a43e

services/core/java/com/android/server/ConnectivityService.java

+16 −26
Original line number Diff line number Diff line
@@ -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;

        }

tests/net/java/com/android/server/ConnectivityServiceTest.java

+2 −10
Original line number Diff line number Diff line
@@ -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