From a5a903d0b59137499217ddf74fc61f8725b9f35a Mon Sep 17 00:00:00 2001
From: Lorenzo Colitti <lorenzo@google.com>
Date: Thu, 4 Feb 2021 00:18:27 +0900
Subject: [PATCH] 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
---
 .../android/server/ConnectivityService.java   | 42 +++++++------------
 .../server/ConnectivityServiceTest.java       | 12 +-----
 2 files changed, 18 insertions(+), 36 deletions(-)

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index f59b9fc4ef..6f0b8b7c0a 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -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;
         }
 
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index d5d7b475fd..0dce9e10e0 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -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